From b83c0747d9dc9c5b92ff227342b3bbef537bda71 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 28 Aug 2017 14:34:04 +0200 Subject: [PATCH] FIX: select-box improvments - more tests for category-select-box - allows to clear selection - fix positioning on safari - focus on click - do not display uncategorized if not allowed in settings - fix component state impacting other specs - better texts - higher category-select-box on mobile --- .../components/category-select-box.js.es6 | 5 + .../discourse/components/select-box.js.es6 | 124 +++++++++--------- .../select-box/select-box-header.js.es6 | 15 +++ .../templates/components/select-box.hbs | 5 +- .../select-box/select-box-header.hbs | 6 + .../common/components/select-box.scss | 26 +++- app/assets/stylesheets/desktop/compose.scss | 4 +- app/assets/stylesheets/mobile/compose.scss | 6 +- config/locales/client.en.yml | 3 +- .../category-select-box-test.js.es6 | 26 ++++ .../components/select-box-test.js.es6 | 22 ++++ 11 files changed, 166 insertions(+), 76 deletions(-) create mode 100644 test/javascripts/acceptance/category-select-box-test.js.es6 diff --git a/app/assets/javascripts/discourse/components/category-select-box.js.es6 b/app/assets/javascripts/discourse/components/category-select-box.js.es6 index eca4c381be1..ae873a01af3 100644 --- a/app/assets/javascripts/discourse/components/category-select-box.js.es6 +++ b/app/assets/javascripts/discourse/components/category-select-box.js.es6 @@ -15,6 +15,8 @@ export default SelectBoxComponent.extend({ width: "100%", + clearable: true, + filterFunction: function() { const _matchFunction = (filter, text) => { return text.toLowerCase().indexOf(filter) > -1; @@ -75,6 +77,9 @@ export default SelectBoxComponent.extend({ const categoryId = c.get("id"); if (scopedCategoryId && categoryId !== scopedCategoryId && c.get("parent_category_id") !== scopedCategoryId) { return false; } if (excludeCategoryId === categoryId) { return false; } + if (!this.siteSettings.allow_uncategorized_topics && c.get("isUncategorizedCategory")) { + return false; + } return c.get("permission") === PermissionType.FULL; }); diff --git a/app/assets/javascripts/discourse/components/select-box.js.es6 b/app/assets/javascripts/discourse/components/select-box.js.es6 index eb12929e76c..3d2b6f0f1f8 100644 --- a/app/assets/javascripts/discourse/components/select-box.js.es6 +++ b/app/assets/javascripts/discourse/components/select-box.js.es6 @@ -1,4 +1,5 @@ import { on, observes } from "ember-addons/ember-computed-decorators"; +import computed from "ember-addons/ember-computed-decorators"; import { iconHTML } from "discourse-common/lib/icon-library"; export default Ember.Component.extend({ @@ -16,9 +17,10 @@ export default Ember.Component.extend({ caretUpIcon: "caret-up", caretDownIcon: "caret-down", - headerText: null, + headerText: I18n.t("select_box.default_header_text"), dynamicHeaderText: true, icon: null, + clearable: false, value: null, selectedContent: null, @@ -103,13 +105,8 @@ export default Ember.Component.extend({ this.set("filterable", false); } - if (this.get("castInteger")) { - this.set("value", parseInt(this.get("value"), 10)); - } - - this.set("headerText", Handlebars.escapeExpression(this.get("headerText"))); - this.setProperties({ + value: this._castInteger(this.get("value")), componentId: this.elementId, filteredContent: [] }); @@ -117,8 +114,7 @@ export default Ember.Component.extend({ @on("willDestroyElement") _removeDocumentListeners: function() { - $(document).off("click.select-box"); - $(document).off("keydown.select-box"); + $(document).off("click.select-box", "keydown.select-box"); $(window).off("resize.select-box"); }, @@ -160,6 +156,8 @@ export default Ember.Component.extend({ _setupDocumentListeners: function() { $(document) .on("click.select-box", (event) => { + if (this.isDestroying || this.isDestroyed) { return; } + const $element = this.$(); const $target = $(event.target); @@ -180,19 +178,13 @@ export default Ember.Component.extend({ @on("didInsertElement") _bindEvents: function() { - this.$(".select-box-offscreen").on("focusin.select-box", () => { - this.set("focused", true); - }); - this.$(".select-box-offscreen").on("focusout.select-box", () => { - this.set("focused", false); - }); + this.$(".select-box-offscreen") + .on("focusin.select-box", () => this.set("focused", true) ) + .on("focusout.select-box", () => this.set("focused", false) ); - this.$(".filter-query").on("focusin.select-box", () => { - this.set("filterFocused", true); - }); - this.$(".filter-query").on("focusout.select-box", () => { - this.set("filterFocused", false); - }); + this.$(".filter-query") + .on("focusin.select-box", () => this.set("filterFocused", true) ) + .on("focusout.select-box", () => this.set("filterFocused", false) ); this.$(".select-box-offscreen").on("keydown.select-box", (event) => { const keyCode = event.keyCode || event.which; @@ -221,18 +213,6 @@ export default Ember.Component.extend({ if (Ember.isNone(this.get("value"))) { this.set("lastHoveredId", null); } - - this.set("filteredContent", this._remapContent(this.get("content"))); - }, - - @observes("filter") - _filterChanged: function() { - if (Ember.isEmpty(this.get("filter"))) { - this.set("filteredContent", this._remapContent(this.get("content"))); - } else { - const filtered = this.filterFunction()(this); - this.set("filteredContent", this._remapContent(filtered)); - } }, @observes("expanded") @@ -250,6 +230,42 @@ export default Ember.Component.extend({ }; }, + @computed("value", "content") + selectedContent(value, content) { + if (Ember.isNone(value)) { + return null; + } + + const selectedContent = content.find((c) => { + return c[this.get("idKey")] === value; + }); + + if (!Ember.isNone(selectedContent)) { + return this._normalizeContent(selectedContent); + } + + return null; + }, + + @computed("headerText", "dynamicHeaderText", "selectedContent.text") + generatedHeadertext(headerText, dynamic, text) { + if (dynamic && !Ember.isNone(text)) { + return text; + } + + return headerText; + }, + + @observes("content.[]", "filter") + _filterChanged: function() { + if (Ember.isEmpty(this.get("filter"))) { + this.set("filteredContent", this._remapContent(this.get("content"))); + } else { + const filtered = this.filterFunction()(this); + this.set("filteredContent", this._remapContent(filtered)); + } + }, + @observes("content.[]", "value") @on("didReceiveAttrs") _contentChanged: function() { @@ -260,13 +276,6 @@ export default Ember.Component.extend({ } this.set("filteredContent", this._remapContent(this.get("content"))); - this._setSelectedContent(this.get("content")); - - if (this.get("dynamicHeaderText")) { - if (!Ember.isNone(this.get("selectedContent.text"))) { - this.set("headerText", this.get("selectedContent.text")); - } - } }, actions: { @@ -274,17 +283,17 @@ export default Ember.Component.extend({ this.toggleProperty("expanded"); }, + onClearSelection() { + this.set("value", null); + }, + onFilterChange(filter) { this.set("filter", filter); }, onSelectRow(id) { - if (this.get("castInteger")) { - id = parseInt(id, 10); - } - this.setProperties({ - value: id, + value: this._castInteger(id), expanded: false }); }, @@ -294,28 +303,13 @@ export default Ember.Component.extend({ } }, - _setSelectedContent(content) { - const selectedContent = content.find((c) => { - return c[this.get("idKey")] === this.get("value"); - }); - - if (!Ember.isNone(selectedContent)) { - this.set("selectedContent", this._normalizeContent(selectedContent)); - } - }, - _remapContent(content) { return content.map(c => this._normalizeContent(c)); }, _normalizeContent(content) { - let id = content[this.get("idKey")]; - if (this.get("castInteger")) { - id = parseInt(id, 10); - } - return { - id, + id: this._castInteger(content[this.get("idKey")]), text: content[this.get("textKey")], icon: content[this.get("iconKey")] }; @@ -330,4 +324,12 @@ export default Ember.Component.extend({ height: headerHeight + this.$(".select-box-body").outerHeight() }); }, + + _castInteger(value) { + if (this.get("castInteger") && Ember.isPresent(value)) { + return parseInt(value, 10); + } + + return value; + } }); diff --git a/app/assets/javascripts/discourse/components/select-box/select-box-header.js.es6 b/app/assets/javascripts/discourse/components/select-box/select-box-header.js.es6 index 57d5f066ab1..2d028d21f99 100644 --- a/app/assets/javascripts/discourse/components/select-box/select-box-header.js.es6 +++ b/app/assets/javascripts/discourse/components/select-box/select-box-header.js.es6 @@ -1,18 +1,33 @@ +import computed from 'ember-addons/ember-computed-decorators'; + export default Ember.Component.extend({ classNames: "select-box-header", classNameBindings: ["focused:is-focused"], + showClearButton: false, + didReceiveAttrs() { this._super(); this._setCaretIcon(); }, + @computed("clearable", "selectedId") + showClearButton(clearable, selectedId) { + return clearable === true && !Ember.isNone(selectedId); + }, + click() { this.sendAction("onToggle"); }, + actions: { + clearSelection() { + this.sendAction("onClearSelection"); + } + }, + _setCaretIcon() { if(this.get("expanded")) { this.set("caretIcon", this.get("caretUpIcon")); diff --git a/app/assets/javascripts/discourse/templates/components/select-box.hbs b/app/assets/javascripts/discourse/templates/components/select-box.hbs index 89fadee0bd2..55b331763b0 100644 --- a/app/assets/javascripts/discourse/templates/components/select-box.hbs +++ b/app/assets/javascripts/discourse/templates/components/select-box.hbs @@ -8,13 +8,16 @@ /> {{component selectBoxHeaderComponent - text=headerText + text=generatedHeadertext focused=focused caretUpIcon=caretUpIcon caretDownIcon=caretDownIcon onToggle=(action "onToggle") + onClearSelection=(action "onClearSelection") icon=icon + clearable=clearable expanded=expanded + selectedId=value }}
diff --git a/app/assets/javascripts/discourse/templates/components/select-box/select-box-header.hbs b/app/assets/javascripts/discourse/templates/components/select-box/select-box-header.hbs index cf3812c0dbe..a44cc1b41b4 100644 --- a/app/assets/javascripts/discourse/templates/components/select-box/select-box-header.hbs +++ b/app/assets/javascripts/discourse/templates/components/select-box/select-box-header.hbs @@ -6,4 +6,10 @@ {{text}} +{{#if showClearButton}} + +{{/if}} + {{d-icon caretIcon class="caret-icon"}} diff --git a/app/assets/stylesheets/common/components/select-box.scss b/app/assets/stylesheets/common/components/select-box.scss index 18c1dcb6a99..b49af983f4b 100644 --- a/app/assets/stylesheets/common/components/select-box.scss +++ b/app/assets/stylesheets/common/components/select-box.scss @@ -91,6 +91,18 @@ -webkit-box-shadow: $tertiary 0px 0px 6px 0px; box-shadow: $tertiary 0px 0px 6px 0px; } + + .clear-selection { + margin: 0 5px; + padding: 0; + outline: none; + + .d-icon { + margin: 0; + padding-left: 10px; + padding-right: 10px; + } + } } .select-box-body { @@ -190,12 +202,6 @@ } } -.select-box .select-box-row { - &.is-highlighted { - background: $highlight-medium; - } -} - .select-box .select-box-collection { -webkit-box-flex: 0; -ms-flex: 0 1 auto; @@ -231,6 +237,14 @@ margin: 0; } + .d-icon { + margin-right: 5px; + } + + &.is-highlighted { + background: $highlight-medium; + } + &.is-selected { a { background: $highlight-medium; diff --git a/app/assets/stylesheets/desktop/compose.scss b/app/assets/stylesheets/desktop/compose.scss index b97538a6cb7..7d4e24fcb19 100644 --- a/app/assets/stylesheets/desktop/compose.scss +++ b/app/assets/stylesheets/desktop/compose.scss @@ -237,8 +237,7 @@ min-width: 1280px; .form-element { position: relative; - display: flex; - + display: inline-block; .category-input { width: 430px; @@ -313,6 +312,7 @@ } .title-input, .category-input, .show-admin-options { display: inline; + float: left; } .show-admin-options { vertical-align: top; diff --git a/app/assets/stylesheets/mobile/compose.scss b/app/assets/stylesheets/mobile/compose.scss index 11d5543eb98..f3515ab2d87 100644 --- a/app/assets/stylesheets/mobile/compose.scss +++ b/app/assets/stylesheets/mobile/compose.scss @@ -128,10 +128,6 @@ input[type=radio], input[type=checkbox] { } .category-input { margin-top: 3px; - - .category-select-box { - height: 28px; - } } .wmd-controls { transition: top 0.3s ease; @@ -206,7 +202,7 @@ input[type=radio], input[type=checkbox] { margin: 7px 0 0 5px; position: absolute; } - .btn-primary { + .create.btn-primary { margin-bottom: 8px; } } diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index c11c8052bb5..d0fad92748e 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1145,7 +1145,8 @@ en: alt: 'Alt' select_box: - no_content: No content + default_header_text: Select... + no_content: No matches found filter_placeholder: Search... emoji_picker: diff --git a/test/javascripts/acceptance/category-select-box-test.js.es6 b/test/javascripts/acceptance/category-select-box-test.js.es6 new file mode 100644 index 00000000000..7d8b9157778 --- /dev/null +++ b/test/javascripts/acceptance/category-select-box-test.js.es6 @@ -0,0 +1,26 @@ +import { acceptance } from "helpers/qunit-helpers"; + +acceptance("CategorySelectBox", { + loggedIn: true, + settings: { + allow_uncategorized_topics: false + } +}); + +QUnit.test("does not display uncategorized if not allowed", assert => { + visit("/"); + click('#create-topic'); + + click(".category-select-box .select-box-header"); + andThen(() => { + assert.ok(!exists('.category-select-box .select-box-row[title="uncategorized"]')); + }); +}); + +QUnit.test("prefill category when category_id is set", assert => { + visit("/new-topic?category_id=1"); + + andThen(() => { + assert.equal(find('.category-select-box .current-selection').html().trim(), "bug"); + }); +}); diff --git a/test/javascripts/components/select-box-test.js.es6 b/test/javascripts/components/select-box-test.js.es6 index 47090c6e687..8bf237af16f 100644 --- a/test/javascripts/components/select-box-test.js.es6 +++ b/test/javascripts/components/select-box-test.js.es6 @@ -313,3 +313,25 @@ componentTest('static headerText', { }); } }); + +componentTest('clearable selection', { + template: '{{select-box value=1 content=content clearable=true}}', + + beforeEach() { + this.set("content", [{ id: 1, text: "robin" }, { id: 2, text: "regis" }]); + }, + + test(assert) { + click(".select-box-header"); + andThen(() => { + assert.ok(exists(".select-box-row.is-highlighted")); + assert.equal(find(".select-box-header .current-selection").html().trim(), "robin"); + }); + + click(".select-box-header .clear-selection"); + andThen(() => { + assert.notOk(exists(".select-box-row.is-highlighted")); + assert.equal(find(".select-box-header .current-selection").html().trim(), "Select..."); + }); + } +});