From be841e666ec0aee601143e26d3612eb0850faa08 Mon Sep 17 00:00:00 2001 From: Krzysztof Kotlarek Date: Thu, 4 Jan 2024 14:29:08 +1100 Subject: [PATCH] FEATURE: filter themes and components (#25105) Allow filtering themes or components to find Active/Enabled Inactive/Disabled or Updates Available in the admin panel. --- .../addon/components/themes-list-item.js | 2 +- .../admin/addon/components/themes-list.hbs | 30 ++-- .../admin/addon/components/themes-list.js | 74 ++++++++-- .../discourse/tests/acceptance/themes-test.js | 2 +- .../components/themes-list-test.js | 133 ++++++++++++++---- .../stylesheets/common/admin/customize.scss | 53 ++++--- config/locales/client.en.yml | 13 +- config/locales/server.en.yml | 4 + 8 files changed, 242 insertions(+), 69 deletions(-) diff --git a/app/assets/javascripts/admin/addon/components/themes-list-item.js b/app/assets/javascripts/admin/addon/components/themes-list-item.js index 7e8466870b3..3630d3a27de 100644 --- a/app/assets/javascripts/admin/addon/components/themes-list-item.js +++ b/app/assets/javascripts/admin/addon/components/themes-list-item.js @@ -8,7 +8,7 @@ import discourseComputed from "discourse-common/utils/decorators"; const MAX_COMPONENTS = 4; -@classNames("themes-list-item") +@classNames("themes-list-container__item") @classNameBindings("theme.selected:selected") export default class ThemesListItem extends Component { childrenExpanded = false; diff --git a/app/assets/javascripts/admin/addon/components/themes-list.hbs b/app/assets/javascripts/admin/addon/components/themes-list.hbs index 3b296e54171..cea7835e7e1 100644 --- a/app/assets/javascripts/admin/addon/components/themes-list.hbs +++ b/app/assets/javascripts/admin/addon/components/themes-list.hbs @@ -17,20 +17,30 @@
- {{#if this.showFilter}} -
+ {{#if this.showSearch}} +
{{d-icon "search"}}
{{/if}} +
+
+ {{i18n "admin.customize.theme.filter_by"}} +
+ +
{{#if this.hasThemes}} - {{#if this.hasActiveThemes}} + {{#if (and this.hasActiveThemes (not this.inactiveFilter))}} {{#each this.activeThemes as |theme|}} {{/each}} - {{#if this.hasInactiveThemes}} -
+ {{#if (and this.hasInactiveThemes (not this.activeFilter))}} +
{{#if this.selectInactiveMode}} @@ -101,7 +111,7 @@ {{/if}} {{/if}} - {{#if this.hasInactiveThemes}} + {{#if (and this.hasInactiveThemes (not this.activeFilter))}} {{#each this.inactiveThemes as |theme|}} +
{{i18n "admin.customize.theme.empty"}}
{{/if}} diff --git a/app/assets/javascripts/admin/addon/components/themes-list.js b/app/assets/javascripts/admin/addon/components/themes-list.js index 32296a7c0ea..8bf160175b5 100644 --- a/app/assets/javascripts/admin/addon/components/themes-list.js +++ b/app/assets/javascripts/admin/addon/components/themes-list.js @@ -5,8 +5,42 @@ import { inject as service } from "@ember/service"; import { classNames } from "@ember-decorators/component"; import DeleteThemesConfirm from "discourse/components/modal/delete-themes-confirm"; import discourseComputed, { bind } from "discourse-common/utils/decorators"; +import I18n from "discourse-i18n"; import { COMPONENTS, THEMES } from "admin/models/theme"; +const ALL_FILTER = "all"; +const ACTIVE_FILTER = "active"; +const INACTIVE_FILTER = "inactive"; +const UPDATES_AVAILABLE_FILTER = "updates_available"; + +const THEMES_FILTERS = [ + { name: I18n.t("admin.customize.theme.all_filter"), id: ALL_FILTER }, + { name: I18n.t("admin.customize.theme.active_filter"), id: ACTIVE_FILTER }, + { + name: I18n.t("admin.customize.theme.inactive_filter"), + id: INACTIVE_FILTER, + }, + { + name: I18n.t("admin.customize.theme.updates_available_filter"), + id: UPDATES_AVAILABLE_FILTER, + }, +]; +const COMPONENTS_FILTERS = [ + { name: I18n.t("admin.customize.component.all_filter"), id: ALL_FILTER }, + { + name: I18n.t("admin.customize.component.enabled_filter"), + id: ACTIVE_FILTER, + }, + { + name: I18n.t("admin.customize.component.disabled_filter"), + id: INACTIVE_FILTER, + }, + { + name: I18n.t("admin.customize.component.updates_available_filter"), + id: UPDATES_AVAILABLE_FILTER, + }, +]; + @classNames("themes-list") export default class ThemesList extends Component { @service router; @@ -14,7 +48,8 @@ export default class ThemesList extends Component { THEMES = THEMES; COMPONENTS = COMPONENTS; - filterTerm = null; + searchTerm = null; + filter = ALL_FILTER; selectInactiveMode = false; @gt("themesList.length", 0) hasThemes; @@ -23,12 +58,15 @@ export default class ThemesList extends Component { @gt("inactiveThemes.length", 0) hasInactiveThemes; - @gte("themesList.length", 10) showFilter; + @gte("themesList.length", 10) showSearch; @equal("currentTab", THEMES) themesTabActive; @equal("currentTab", COMPONENTS) componentsTabActive; + @equal("filter", ACTIVE_FILTER) activeFilter; + @equal("filter", INACTIVE_FILTER) inactiveFilter; + @discourseComputed("themes", "components", "currentTab") themesList(themes, components) { if (this.themesTabActive) { @@ -38,13 +76,23 @@ export default class ThemesList extends Component { } } + @discourseComputed("currentTab") + selectableFilters() { + if (this.themesTabActive) { + return THEMES_FILTERS; + } else { + return COMPONENTS_FILTERS; + } + } + @discourseComputed( "themesList", "currentTab", "themesList.@each.user_selectable", "themesList.@each.default", "themesList.@each.markedToDelete", - "filterTerm" + "searchTerm", + "filter" ) inactiveThemes(themes) { let results; @@ -57,7 +105,10 @@ export default class ThemesList extends Component { (theme) => !theme.get("user_selectable") && !theme.get("default") ); } - return this._filterThemes(results, this.filterTerm); + if (this.filter === UPDATES_AVAILABLE_FILTER) { + results = results.filterBy("isPendingUpdates"); + } + return this._searchThemes(results, this.searchTerm); } @discourseComputed("themesList.@each.markedToDelete") @@ -75,7 +126,8 @@ export default class ThemesList extends Component { "currentTab", "themesList.@each.user_selectable", "themesList.@each.default", - "filterTerm" + "searchTerm", + "filter" ) activeThemes(themes) { let results; @@ -96,7 +148,10 @@ export default class ThemesList extends Component { .localeCompare(b.get("name").toLowerCase()); }); } - return this._filterThemes(results, this.filterTerm); + if (this.filter === UPDATES_AVAILABLE_FILTER) { + results = results.filterBy("isPendingUpdates"); + } + return this._searchThemes(results, this.searchTerm); } @discourseComputed("themesList.@each.markedToDelete") someInactiveSelected() { @@ -111,7 +166,7 @@ export default class ThemesList extends Component { return this.selectedCount === this.inactiveThemes.length; } - _filterThemes(themes, term) { + _searchThemes(themes, term) { term = term?.trim()?.toLowerCase(); if (!term) { return themes; @@ -131,8 +186,9 @@ export default class ThemesList extends Component { if (newTab !== this.currentTab) { this.set("selectInactiveMode", false); this.set("currentTab", newTab); - if (!this.showFilter) { - this.set("filterTerm", null); + this.set("filter", ALL_FILTER); + if (!this.showSearch) { + this.set("searchTerm", null); } } } diff --git a/app/assets/javascripts/discourse/tests/acceptance/themes-test.js b/app/assets/javascripts/discourse/tests/acceptance/themes-test.js index f972c5ee1fa..34acbde0e13 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/themes-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/themes-test.js @@ -215,7 +215,7 @@ acceptance("Theme", function (needs) { test("can continue installation", async function (assert) { await visit("/admin/customize/themes"); - await click(".themes-list-container .themes-list-item"); + await click(".themes-list-container__item .info"); assert.ok( query(".control-unit .status-message").innerText.includes( I18n.t("admin.customize.theme.last_attempt") diff --git a/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js b/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js index cbd23d8cdb8..ff7b25921c5 100644 --- a/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js +++ b/app/assets/javascripts/discourse/tests/integration/components/themes-list-test.js @@ -8,6 +8,7 @@ import { query, queryAll, } from "discourse/tests/helpers/qunit-helpers"; +import selectKit from "discourse/tests/helpers/select-kit-helper"; import I18n from "discourse-i18n"; import Theme, { COMPONENTS, THEMES } from "admin/models/theme"; @@ -59,14 +60,18 @@ module("Integration | Component | themes-list", function (hooks) { exists(".inactive-indicator"), "there is no inactive themes separator when all themes are inactive" ); - assert.strictEqual(count(".themes-list-item"), 5, "displays all themes"); + assert.strictEqual( + count(".themes-list-container__item .info"), + 5, + "displays all themes" + ); [2, 3].forEach((num) => this.themes[num].set("user_selectable", true)); this.themes[4].set("default", true); this.set("themes", this.themes); const names = [4, 2, 3, 0, 1].map((num) => this.themes[num].get("name")); // default theme always on top, followed by user-selectable ones and then the rest assert.deepEqual( - Array.from(queryAll(".themes-list-item .name")).map((node) => + [...queryAll(".themes-list-container__item .info .name")].map((node) => node.innerText.trim() ), names, @@ -74,7 +79,7 @@ module("Integration | Component | themes-list", function (hooks) { ); assert.strictEqual( queryAll(".inactive-indicator").index(), - 3, + 4, "the separator is in the right location" ); @@ -87,12 +92,12 @@ module("Integration | Component | themes-list", function (hooks) { this.set("themes", []); assert.strictEqual( - count(".themes-list-item"), + count(".themes-list-container__item .empty"), 1, "shows one entry with a message when there is nothing to display" ); assert.strictEqual( - query(".themes-list-item span.empty").innerText.trim(), + query(".themes-list-container__item span.empty").innerText.trim(), I18n.t("admin.customize.theme.empty"), "displays the right message" ); @@ -131,25 +136,25 @@ module("Integration | Component | themes-list", function (hooks) { assert.notOk(exists(".inactive-indicator"), "there is no separator"); assert.strictEqual( - count(".themes-list-item"), + count(".themes-list-container__item .info"), 5, "displays all components" ); this.set("components", []); assert.strictEqual( - count(".themes-list-item"), + count(".themes-list-container__item .empty"), 1, "shows one entry with a message when there is nothing to display" ); assert.strictEqual( - query(".themes-list-item span.empty").innerText.trim(), + query(".themes-list-container__item span.empty").innerText.trim(), I18n.t("admin.customize.theme.empty"), "displays the right message" ); }); - test("themes filter is not visible when there are less than 10 themes", async function (assert) { + test("themes search is not visible when there are less than 10 themes", async function (assert) { const themes = createThemes(9); this.setProperties({ themes, @@ -161,12 +166,12 @@ module("Integration | Component | themes-list", function (hooks) { ); assert.ok( - !exists(".themes-list-filter"), - "filter input not shown when we have fewer than 10 themes" + !exists(".themes-list-search"), + "search input not shown when we have fewer than 10 themes" ); }); - test("themes filter keeps themes whose names include the filter term", async function (assert) { + test("themes search keeps themes whose names include the search term", async function (assert) { const themes = ["osama", "OsAmaa", "osAMA 1234"] .map((name) => Theme.create({ name: `Theme ${name}` })) .concat(createThemes(7)); @@ -179,18 +184,90 @@ module("Integration | Component | themes-list", function (hooks) { hbs`` ); - assert.ok(exists(".themes-list-filter")); - await fillIn(".themes-list-filter .filter-input", " oSAma "); + assert.ok(exists(".themes-list-container__search-input")); + await fillIn(".themes-list-container__search-input", " oSAma "); assert.deepEqual( - Array.from(queryAll(".themes-list-item .name")).map((node) => + [...queryAll(".themes-list-container__item .info .name")].map((node) => node.textContent.trim() ), ["Theme osama", "Theme OsAmaa", "Theme osAMA 1234"], - "only themes whose names include the filter term are shown" + "only themes whose names include the search term are shown" ); }); - test("switching between themes and components tabs keeps the filter visible only if both tabs have at least 10 items", async function (assert) { + test("themes filter", async function (assert) { + const themes = [ + Theme.create({ name: "Theme enabled 1", user_selectable: true }), + Theme.create({ name: "Theme enabled 2", user_selectable: true }), + Theme.create({ name: "Theme disabled 1", user_selectable: false }), + Theme.create({ + name: "Theme disabled 2", + user_selectable: false, + remote_theme: { + id: 42, + remote_url: + "git@github.com:discourse-org/discourse-incomplete-theme.git", + commits_behind: 1, + }, + }), + ]; + this.setProperties({ + themes, + currentTab: THEMES, + }); + + await render( + hbs`` + ); + + assert.ok(exists(".themes-list-container__filter-input")); + assert.deepEqual( + [...queryAll(".themes-list-container__item .info .name")].map((node) => + node.textContent.trim() + ), + [ + "Theme enabled 1", + "Theme enabled 2", + "Theme disabled 1", + "Theme disabled 2", + ] + ); + + await selectKit(".themes-list-container__filter-input").expand(); + await selectKit(".themes-list-container__filter-input").selectRowByValue( + "active" + ); + assert.deepEqual( + [...queryAll(".themes-list-container__item .info .name")].map((node) => + node.textContent.trim() + ), + ["Theme enabled 1", "Theme enabled 2"] + ); + + await selectKit(".themes-list-container__filter-input").expand(); + await selectKit(".themes-list-container__filter-input").selectRowByValue( + "inactive" + ); + assert.deepEqual( + [...queryAll(".themes-list-container__item .info .name")].map((node) => + node.textContent.trim() + ), + ["Theme disabled 1", "Theme disabled 2"] + ); + + await selectKit(".themes-list-container__filter-input").expand(); + await selectKit(".themes-list-container__filter-input").selectRowByValue( + "updates_available" + ); + assert.deepEqual( + [...queryAll(".themes-list-container__item .info .name")].map((node) => + node.textContent.trim() + ), + ["Theme disabled 2"] + ); + }); + + test("switching between themes and components tabs keeps the search visible only if both tabs have at least 10 items", async function (assert) { const themes = createThemes(10, (n) => { return { name: `Theme ${n}${n}` }; }); @@ -212,20 +289,20 @@ module("Integration | Component | themes-list", function (hooks) { hbs`` ); - await fillIn(".themes-list-filter .filter-input", "11"); + await fillIn(".themes-list-container__search-input", "11"); assert.strictEqual( - query(".themes-list-container").textContent.trim(), + query(".themes-list-container__item .info").textContent.trim(), "Theme 11", "only 1 theme is shown" ); await click(".themes-list-header .components-tab"); assert.ok( - !exists(".themes-list-filter"), - "filter input/term do not persist when we switch to the other" + + !exists(".themes-list-container__search-input"), + "search input/term do not persist when we switch to the other" + " tab because it has fewer than 10 items" ); assert.deepEqual( - Array.from(queryAll(".themes-list-item .name")).map((node) => + [...queryAll(".themes-list-container__item .info .name")].map((node) => node.textContent.trim() ), [ @@ -253,22 +330,22 @@ module("Integration | Component | themes-list", function (hooks) { ) ); assert.ok( - exists(".themes-list-filter"), - "filter is now shown for the components tab" + exists(".themes-list-container__search-input"), + "search is now shown for the components tab" ); - await fillIn(".themes-list-filter .filter-input", "66"); + await fillIn(".themes-list-container__search-input", "66"); assert.strictEqual( - query(".themes-list-container").textContent.trim(), + query(".themes-list-container__item .info").textContent.trim(), "Component 66", "only 1 component is shown" ); await click(".themes-list-header .themes-tab"); assert.strictEqual( - query(".themes-list-container").textContent.trim(), + query(".themes-list-container__item .info").textContent.trim(), "Theme 66", - "filter term persisted between tabs because both have more than 10 items" + "search term persisted between tabs because both have more than 10 items" ); }); }); diff --git a/app/assets/stylesheets/common/admin/customize.scss b/app/assets/stylesheets/common/admin/customize.scss index 8d71216c03d..7c29ccd53f9 100644 --- a/app/assets/stylesheets/common/admin/customize.scss +++ b/app/assets/stylesheets/common/admin/customize.scss @@ -246,7 +246,6 @@ } .themes-list-container { - overflow-y: auto; box-sizing: border-box; max-height: 60vh; border-bottom-right-radius: var(--d-border-radius); @@ -262,10 +261,10 @@ border-left: 1px solid var(--primary-low); width: 100%; - .themes-list-item:last-child { + &__item:last-child { border-bottom: none; } - .themes-list-item { + &__item { color: var(--primary); border-bottom: 1px solid var(--primary-low); display: flex; @@ -391,34 +390,52 @@ width: 100%; } } + &__filter { + padding-left: 0.67em; + display: flex; + height: 3em; + align-items: center; + background-color: var(--primary-very-low); + } - .themes-list-filter { + &__filter-label { + white-space: nowrap; + margin-right: 1em; + } + &__filter-input { + margin-right: 0.5em; + summary { + width: auto; + } + } + + &__search { display: flex; align-items: center; position: sticky; top: 0; - background: var(--secondary); z-index: z("base"); height: 3em; + background: var(--primary-very-low); .d-icon { position: absolute; padding-left: 0.5em; } + } + &__search-input { + width: 100%; + height: 100%; + margin: 0; + border: 0; + padding-left: 2em; + background-color: var(--primary-very-low); - .filter-input { - width: 100%; - height: 100%; - margin: 0; - border: 0; - padding-left: 2em; + &:focus { + outline: 0; - &:focus { - outline: 0; - - ~ .d-icon { - color: var(--tertiary-hover); - } + ~ .d-icon { + color: var(--tertiary-hover); } } } @@ -483,7 +500,7 @@ left: 0; right: 0; z-index: z("fullscreen"); - background-color: var(--secondary); + background: var(--secondary); width: 100%; padding: 0; margin: 0; diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 544879944de..5c9589d7c69 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5278,12 +5278,17 @@ en: body: "Body" revert: "Revert Changes" revert_confirm: "Are you sure you want to revert your changes?" - + component: + all_filter: "All" + enabled_filter: "Enabled" + disabled_filter: "Disabled" + updates_available_filter: "Updates Available" theme: + filter_by: "Filter by" theme: "Theme" component: "Component" components: "Components" - filter_placeholder: "type to filter…" + search_placeholder: "type to search…" theme_name: "Theme name" component_name: "Component name" themes_intro: "Select an existing theme or install a new one to get started" @@ -5457,6 +5462,10 @@ en: title: "Define theme settings in YAML format" scss_color_variables_warning: 'Using core SCSS color variables in themes is deprecated. Please use CSS custom properties instead. See this guide for more details.' scss_warning_inline: "Using core SCSS color variables in themes is deprecated." + all_filter: "All" + active_filter: "Active" + inactive_filter: "Inactive" + updates_available_filter: "Updates Available" colors: select_base: title: "Select base color palette" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 1885d5c984e..c3c880bbab0 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -70,6 +70,10 @@ en: inline_oneboxer: topic_page_title_post_number: "#%{post_number}" topic_page_title_post_number_by_user: "#%{post_number} by %{username}" + components: + enabled_filter: "Enabled" + disabled_filter: "Disabled" + updates_available_filter: "Updates Available" themes: bad_color_scheme: "Can not update theme, invalid color palette" other_error: "Something went wrong updating theme"