From 9911a41f4c98a935574a5df9516d932c8e72d923 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Thu, 15 Nov 2018 15:21:40 +0100 Subject: [PATCH] FIX: improves category/tag drops header shortcuts (#6610) - wont appear when filtering - can now be selected with keyboard - fix bugs on click with safari/firefox --- .../components/category-drop.js.es6 | 88 +++++++++---------- .../select-kit/components/multi-select.js.es6 | 5 ++ .../select-kit/components/select-kit.js.es6 | 4 +- .../components/single-select.js.es6 | 5 ++ .../select-kit/components/tag-drop.js.es6 | 77 +++++++++------- app/assets/stylesheets/common.scss | 1 + .../common/select-kit/category-drop.scss | 6 ++ .../common/select-kit/tag-drop.scss | 13 +++ .../components/single-select-test.js.es6 | 23 +++++ .../components/tag-drop-test.js.es6 | 6 +- 10 files changed, 145 insertions(+), 83 deletions(-) create mode 100644 app/assets/stylesheets/common/select-kit/tag-drop.scss diff --git a/app/assets/javascripts/select-kit/components/category-drop.js.es6 b/app/assets/javascripts/select-kit/components/category-drop.js.es6 index 96343a748a9..fdaa6d10a1e 100644 --- a/app/assets/javascripts/select-kit/components/category-drop.js.es6 +++ b/app/assets/javascripts/select-kit/components/category-drop.js.es6 @@ -10,7 +10,7 @@ export default ComboBoxComponent.extend({ classNameBindings: ["categoryStyle"], classNames: "category-drop", verticalOffset: 3, - content: Ember.computed.alias("categories"), + content: Ember.computed.alias("categoriesWithShortcuts"), rowComponent: "category-row", headerComponent: "category-drop/category-drop-header", allowAutoSelectFirst: false, @@ -24,6 +24,34 @@ export default ComboBoxComponent.extend({ subCategory: false, isAsync: Ember.computed.not("subCategory"), + @computed("categories", "hasSelection", "subCategory", "noSubcategories") + categoriesWithShortcuts( + categories, + hasSelection, + subCategory, + noSubcategories + ) { + const shortcuts = []; + + if (hasSelection || (noSubcategories && subCategory)) { + shortcuts.push({ + name: this.get("allCategoriesLabel"), + __sk_row_type: "noopRow", + id: "all-categories" + }); + } + + if (subCategory && (hasSelection || !noSubcategories)) { + shortcuts.push({ + name: this.get("noCategoriesLabel"), + __sk_row_type: "noopRow", + id: "no-categories" + }); + } + + return shortcuts.concat(categories); + }, + init() { this._super(); @@ -54,45 +82,6 @@ export default ComboBoxComponent.extend({ ); }, - @computed( - "allCategoriesUrl", - "allCategoriesLabel", - "noCategoriesUrl", - "noCategoriesLabel" - ) - collectionHeader( - allCategoriesUrl, - allCategoriesLabel, - noCategoriesUrl, - noCategoriesLabel - ) { - let shortcuts = ""; - - if ( - this.get("hasSelection") || - (this.get("noSubcategories") && this.get("subCategory")) - ) { - shortcuts += ` - - ${allCategoriesLabel} - - `; - } - - if ( - this.get("subCategory") && - (this.get("hasSelection") || !this.get("noSubcategories")) - ) { - shortcuts += ` - - ${noCategoriesLabel} - - `; - } - - return shortcuts.htmlSafe(); - }, - computeHeaderContent() { let content = this._super(); @@ -131,19 +120,28 @@ export default ComboBoxComponent.extend({ @computed("parentCategory.url", "subCategory") allCategoriesUrl(parentCategoryUrl, subCategory) { - return subCategory ? parentCategoryUrl || "/" : "/"; + return Discourse.getURL(subCategory ? parentCategoryUrl || "/" : "/"); }, @computed("parentCategory.url") noCategoriesUrl(parentCategoryUrl) { - return `${parentCategoryUrl}/none`; + return Discourse.getURL(`${parentCategoryUrl}/none`); }, actions: { onSelect(categoryId) { - const category = Category.findById(parseInt(categoryId, 10)); - const categoryURL = - Discourse.getURL("/c/") + Discourse.Category.slugFor(category); + let categoryURL; + + if (categoryId === "all-categories") { + categoryURL = Discourse.getURL(this.get("allCategoriesUrl")); + } else if (categoryId === "no-categories") { + categoryURL = Discourse.getURL(this.get("noCategoriesUrl")); + } else { + const category = Category.findById(parseInt(categoryId, 10)); + const slug = Discourse.Category.slugFor(category); + categoryURL = Discourse.getURL("/c/") + slug; + } + DiscourseURL.routeTo(categoryURL); }, diff --git a/app/assets/javascripts/select-kit/components/multi-select.js.es6 b/app/assets/javascripts/select-kit/components/multi-select.js.es6 index c96f0033dae..2dc640824ac 100644 --- a/app/assets/javascripts/select-kit/components/multi-select.js.es6 +++ b/app/assets/javascripts/select-kit/components/multi-select.js.es6 @@ -264,6 +264,11 @@ export default SelectKitComponent.extend({ return; } + if (computedContentItem.__sk_row_type === "noopRow") { + this._boundaryActionHandler("onSelect", computedContentItem.value); + return; + } + if (computedContentItem.__sk_row_type === "createRow") { if ( !this.get("computedValues").includes(computedContentItem.value) && diff --git a/app/assets/javascripts/select-kit/components/select-kit.js.es6 b/app/assets/javascripts/select-kit/components/select-kit.js.es6 index b69c1d5d049..36d1a4ca1ab 100644 --- a/app/assets/javascripts/select-kit/components/select-kit.js.es6 +++ b/app/assets/javascripts/select-kit/components/select-kit.js.es6 @@ -213,7 +213,9 @@ export default Ember.Component.extend( name: name || this._nameForContent(contentItem), locked: false, created: options.created || false, - __sk_row_type: options.created ? "createRow" : null, + __sk_row_type: options.created + ? "createRow" + : contentItem.__sk_row_type, originalContent }; diff --git a/app/assets/javascripts/select-kit/components/single-select.js.es6 b/app/assets/javascripts/select-kit/components/single-select.js.es6 index af3aa3a4f8d..4e18f7382b7 100644 --- a/app/assets/javascripts/select-kit/components/single-select.js.es6 +++ b/app/assets/javascripts/select-kit/components/single-select.js.es6 @@ -210,6 +210,11 @@ export default SelectKitComponent.extend({ }, select(computedContentItem) { + if (computedContentItem.__sk_row_type === "noopRow") { + this._boundaryActionHandler("onSelect", computedContentItem.value); + return; + } + if (this.get("hasSelection")) { this.deselect(this.get("selection.value")); } diff --git a/app/assets/javascripts/select-kit/components/tag-drop.js.es6 b/app/assets/javascripts/select-kit/components/tag-drop.js.es6 index c99806b8481..25f20d69d77 100644 --- a/app/assets/javascripts/select-kit/components/tag-drop.js.es6 +++ b/app/assets/javascripts/select-kit/components/tag-drop.js.es6 @@ -67,9 +67,9 @@ export default ComboBoxComponent.extend(TagsMixin, { @computed("firstCategory", "secondCategory") allTagsUrl() { if (this.get("currentCategory")) { - return this.get("currentCategory.url") + "?allTags=1"; + return Discourse.getURL(this.get("currentCategory.url") + "?allTags=1"); } else { - return "/"; + return Discourse.getURL("/"); } }, @@ -79,30 +79,7 @@ export default ComboBoxComponent.extend(TagsMixin, { if (this.get("currentCategory")) { url += this.get("currentCategory.url"); } - return `${url}/none`; - }, - - @computed("allTagsUrl", "allTagsLabel", "noTagsUrl", "noTagsLabel") - collectionHeader(allTagsUrl, allTagsLabel, noTagsUrl, noTagsLabel) { - let content = ""; - - if (this.get("tagId") !== "none") { - content += ` - - ${noTagsLabel} - - `; - } - - if (this.get("tagId")) { - content += ` - - ${allTagsLabel} - - `; - } - - return content; + return Discourse.getURL(`${url}/none`); }, @computed("tag") @@ -115,12 +92,35 @@ export default ComboBoxComponent.extend(TagsMixin, { return I18n.t("tagging.selector_no_tags"); }, - @computed("site.top_tags") - content(topTags) { + @computed("tagId", "allTagsLabel", "noTagsLabel") + shortcuts(tagId, allTagsLabel, noTagsLabel) { + const shortcuts = []; + + if (tagId !== "none") { + shortcuts.push({ + name: noTagsLabel, + __sk_row_type: "noopRow", + id: "no-tags" + }); + } + + if (tagId) { + shortcuts.push({ + name: allTagsLabel, + __sk_row_type: "noopRow", + id: "all-tags" + }); + } + + return shortcuts; + }, + + @computed("site.top_tags", "shortcuts") + content(topTags, shortcuts) { if (this.siteSettings.tags_sort_alphabetically && topTags) { - return topTags.sort(); + return shortcuts.concat(topTags.sort()); } else { - return topTags; + return shortcuts.concat(topTags); } }, @@ -141,11 +141,20 @@ export default ComboBoxComponent.extend(TagsMixin, { actions: { onSelect(tagId) { - let url = "/tags"; - if (this.get("currentCategory")) { - url += this.get("currentCategory.url"); + let url; + + if (tagId === "all-tags") { + url = Discourse.getURL(this.get("allTagsUrl")); + } else if (tagId === "no-tags") { + url = Discourse.getURL(this.get("noTagsUrl")); + } else { + url = "/tags"; + if (this.get("currentCategory")) { + url += this.get("currentCategory.url"); + } + url = Discourse.getURL(`${url}/${tagId.toLowerCase()}`); } - url = `${url}/${tagId.toLowerCase()}`; + DiscourseURL.routeTo(url); }, diff --git a/app/assets/stylesheets/common.scss b/app/assets/stylesheets/common.scss index d1c9e353363..6c586c878b1 100644 --- a/app/assets/stylesheets/common.scss +++ b/app/assets/stylesheets/common.scss @@ -24,6 +24,7 @@ @import "common/select-kit/pinned-button"; @import "common/select-kit/select-kit"; @import "common/select-kit/tag-chooser"; +@import "common/select-kit/tag-drop"; @import "common/select-kit/toolbar-popup-menu-options"; @import "common/select-kit/topic-notifications-button"; @import "common/components/*"; diff --git a/app/assets/stylesheets/common/select-kit/category-drop.scss b/app/assets/stylesheets/common/select-kit/category-drop.scss index dd5a59fe6c8..71d1866acea 100644 --- a/app/assets/stylesheets/common/select-kit/category-drop.scss +++ b/app/assets/stylesheets/common/select-kit/category-drop.scss @@ -42,6 +42,12 @@ flex-direction: column; align-items: flex-start; + &[data-value="all-categories"], + &[data-value="no-categories"] { + color: $tertiary; + font-weight: 700; + } + .category-desc { font-weight: normal; color: $primary-medium; diff --git a/app/assets/stylesheets/common/select-kit/tag-drop.scss b/app/assets/stylesheets/common/select-kit/tag-drop.scss new file mode 100644 index 00000000000..6fefb1dcd8b --- /dev/null +++ b/app/assets/stylesheets/common/select-kit/tag-drop.scss @@ -0,0 +1,13 @@ +.select-kit { + &.combo-box { + &.tag-drop { + .select-kit-row { + &[data-value="all-tags"], + &[data-value="no-tags"] { + color: $tertiary; + font-weight: 700; + } + } + } + } +} diff --git a/test/javascripts/components/single-select-test.js.es6 b/test/javascripts/components/single-select-test.js.es6 index 72366fdf72e..a56157b5804 100644 --- a/test/javascripts/components/single-select-test.js.es6 +++ b/test/javascripts/components/single-select-test.js.es6 @@ -887,3 +887,26 @@ componentTest("onDeselect", { ); } }); + +componentTest("noopRow", { + template: "{{single-select value=value content=content}}", + + beforeEach() { + this.set("value", "blue"); + this.set("content", [ + { id: "red", name: "Red", __sk_row_type: "noopRow" }, + "blue", + "green" + ]); + }, + + async test(assert) { + await this.get("subject").expand(); + await this.get("subject").selectRowByValue("red"); + assert.equal(this.get("value"), "blue", "it doesn’t change the value"); + + await this.get("subject").expand(); + await this.get("subject").selectRowByValue("green"); + assert.equal(this.get("value"), "green"); + } +}); diff --git a/test/javascripts/components/tag-drop-test.js.es6 b/test/javascripts/components/tag-drop-test.js.es6 index 5859f845c73..c815abfb786 100644 --- a/test/javascripts/components/tag-drop-test.js.es6 +++ b/test/javascripts/components/tag-drop-test.js.es6 @@ -42,7 +42,7 @@ componentTest("default", { assert.equal( this.get("subject") - .rowByIndex(0) + .rowByIndex(1) .name(), "jeff", "it has the correct tag" @@ -50,7 +50,7 @@ componentTest("default", { assert.equal( this.get("subject") - .rowByIndex(1) + .rowByIndex(2) .name(), "neil", "it has the correct tag" @@ -68,7 +68,7 @@ componentTest("default", { await this.get("subject").fillInFilter(""); assert.equal( this.get("subject") - .rowByIndex(0) + .rowByIndex(1) .name(), "jeff", "it returns top tags for an empty search"