diff --git a/app/assets/javascripts/admin/addon/components/themes-list.js b/app/assets/javascripts/admin/addon/components/themes-list.js index c70d373f4d2..77172ff458d 100644 --- a/app/assets/javascripts/admin/addon/components/themes-list.js +++ b/app/assets/javascripts/admin/addon/components/themes-list.js @@ -77,6 +77,13 @@ export default class ThemesList extends Component { @equal("filter", ACTIVE_FILTER) activeFilter; @equal("filter", INACTIVE_FILTER) inactiveFilter; + willRender() { + super.willRender(...arguments); + if (!this.showSearchAndFilter) { + this.set("searchTerm", null); + } + } + @discourseComputed("themes", "components", "currentTab") themesList(themes, components) { if (this.themesTabActive) { @@ -208,11 +215,8 @@ export default class ThemesList extends Component { changeView(newTab) { if (newTab !== this.currentTab) { this.set("selectInactiveMode", false); - this.set("currentTab", newTab); this.set("filter", ALL_FILTER); - if (!this.showSearchAndFilter) { - this.set("searchTerm", null); - } + this.router.transitionTo("adminCustomizeThemes", newTab); } } diff --git a/app/assets/javascripts/admin/addon/routes/admin-customize-theme-components.js b/app/assets/javascripts/admin/addon/routes/admin-customize-theme-components.js deleted file mode 100644 index 964b1e94b4d..00000000000 --- a/app/assets/javascripts/admin/addon/routes/admin-customize-theme-components.js +++ /dev/null @@ -1,14 +0,0 @@ -import Route from "@ember/routing/route"; -import { service } from "@ember/service"; -import { COMPONENTS } from "admin/models/theme"; - -export default class AdminCustomizeThemeComponents extends Route { - @service router; - - beforeModel(transition) { - transition.abort(); - this.router.transitionTo("adminCustomizeThemes", { - queryParams: { tab: COMPONENTS }, - }); - } -} diff --git a/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js b/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js index 947fe139994..107110ca2d0 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js +++ b/app/assets/javascripts/admin/addon/routes/admin-customize-themes.js @@ -13,23 +13,20 @@ export default class AdminCustomizeThemesRoute extends Route { queryParams = { repoUrl: null, repoName: null, - tab: null, }; - model() { + model(params) { + this.currentTab = params.type; return this.store.findAll("theme"); } setupController(controller, model) { super.setupController(controller, model); - if (controller.tab) { + if (this.currentTab) { controller.setProperties({ editingTheme: false, - currentTab: controller.tab, - - // this is to get rid of the queryString since we don't want it hanging around - tab: undefined, + currentTab: this.currentTab, }); } diff --git a/app/assets/javascripts/admin/addon/routes/admin-route-map.js b/app/assets/javascripts/admin/addon/routes/admin-route-map.js index 41faefcc324..94be1effbed 100644 --- a/app/assets/javascripts/admin/addon/routes/admin-route-map.js +++ b/app/assets/javascripts/admin/addon/routes/admin-route-map.js @@ -55,7 +55,7 @@ export default function () { this.route( "adminCustomizeThemes", - { path: "themes", resetNamespace: true }, + { path: "/:type", resetNamespace: true }, function () { this.route("show", { path: "/:theme_id" }, function () { this.route("schema", { path: "schema/:setting_name" }); @@ -64,11 +64,6 @@ export default function () { } ); - this.route("adminCustomizeThemeComponents", { - path: "theme-components", - resetNamespace: true, - }); - this.route( "adminSiteText", { path: "/site_texts", resetNamespace: true }, diff --git a/app/assets/javascripts/admin/addon/templates/admin.hbs b/app/assets/javascripts/admin/addon/templates/admin.hbs index 5342d148c0d..7742b71b971 100644 --- a/app/assets/javascripts/admin/addon/templates/admin.hbs +++ b/app/assets/javascripts/admin/addon/templates/admin.hbs @@ -25,7 +25,11 @@ {{/if}} - + {{#if this.currentUser.admin}} {{#if this.siteSettings.enable_backups}} diff --git a/app/assets/javascripts/admin/addon/templates/customize.hbs b/app/assets/javascripts/admin/addon/templates/customize.hbs index 3de339ef482..f2be8dc4945 100644 --- a/app/assets/javascripts/admin/addon/templates/customize.hbs +++ b/app/assets/javascripts/admin/addon/templates/customize.hbs @@ -3,6 +3,7 @@ {{#if this.currentUser.admin}} diff --git a/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js b/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js index c45ba2cdf0f..4d0c7a0bc2f 100644 --- a/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js +++ b/app/assets/javascripts/discourse/app/lib/sidebar/admin-nav-map.js @@ -150,10 +150,18 @@ export const ADMIN_NAV_MAP = [ { name: "admin_themes", route: "adminCustomizeThemes", - query: { tab: "themes" }, + routeModels: ["themes"], + model: "themes", label: "admin.appearance.sidebar_link.themes", icon: "paint-brush", }, + { + name: "admin_components", + route: "adminCustomizeThemes", + routeModels: ["components"], + label: "admin.appearance.sidebar_link.components", + icon: "puzzle-piece", + }, { name: "admin_customize_site_texts", route: "adminSiteText", 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 f061e20350b..bf39fb01c27 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 @@ -1,4 +1,4 @@ -import { click, fillIn, render } from "@ember/test-helpers"; +import { fillIn, render } from "@ember/test-helpers"; import { hbs } from "ember-cli-htmlbars"; import { module, test } from "qunit"; import { setupRenderingTest } from "discourse/tests/helpers/component-test"; @@ -367,86 +367,4 @@ module("Integration | Component | themes-list", function (hooks) { ); assert.deepEqual(componentNames(), ["Component unused and disabled 1"]); }); - - 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}` }; - }); - const components = createThemes(5, (n) => { - return { - name: `Component ${n}${n}`, - component: true, - parent_themes: [1], - parentThemes: [1], - }; - }); - this.setProperties({ - themes, - components, - currentTab: THEMES, - }); - - await render( - hbs`` - ); - - await fillIn(".themes-list-search__input", "11"); - assert.strictEqual( - 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-search__input"), - "search input/term do not persist when we switch to the other" + - " tab because it has fewer than 10 items" - ); - assert.deepEqual( - [...queryAll(".themes-list-container__item .info .name")].map((node) => - node.textContent.trim() - ), - [ - "Component 11", - "Component 22", - "Component 33", - "Component 44", - "Component 55", - ], - "all components are shown" - ); - - this.set( - "components", - this.components.concat( - createThemes(5, (n) => { - n += 5; - return { - name: `Component ${n}${n}`, - component: true, - parent_themes: [1], - parentThemes: [1], - }; - }) - ) - ); - assert.ok( - exists(".themes-list-search__input"), - "search is now shown for the components tab" - ); - - await fillIn(".themes-list-search__input", "66"); - assert.strictEqual( - 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__item .info").textContent.trim(), - "Theme 66", - "search term persisted between tabs because both have more than 10 items" - ); - }); }); diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 0bbb98fea3a..b816492e1ef 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -5337,6 +5337,7 @@ en: emoji: "Emoji" navigation: "Navigation" themes: "Themes" + components: "Components" site_texts: "Site Texts" email_settings: diff --git a/config/routes.rb b/config/routes.rb index 63c73ce2a87..3f1de84dbcd 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -221,6 +221,7 @@ Discourse::Application.routes.draw do get "customize" => "color_schemes#index", :constraints => AdminConstraint.new get "customize/themes" => "themes#index", :constraints => AdminConstraint.new + get "customize/components" => "themes#index", :constraints => AdminConstraint.new get "customize/theme-components" => "themes#index", :constraints => AdminConstraint.new get "customize/colors" => "color_schemes#index", :constraints => AdminConstraint.new get "customize/colors/:id" => "color_schemes#index", :constraints => AdminConstraint.new @@ -257,6 +258,7 @@ Discourse::Application.routes.draw do get "themes/:id/:target/:field_name/edit" => "themes#index" get "themes/:id" => "themes#index" + get "components/:id" => "themes#index" get "themes/:id/export" => "themes#export" get "themes/:id/schema/:setting_name" => "themes#schema" diff --git a/spec/system/admin_customize_themes_spec.rb b/spec/system/admin_customize_themes_spec.rb index dd6a43210f5..be62b4b6da9 100644 --- a/spec/system/admin_customize_themes_spec.rb +++ b/spec/system/admin_customize_themes_spec.rb @@ -48,9 +48,35 @@ describe "Admin Customize Themes", type: :system do end it "selects the component tab when visiting the theme-components route" do - visit("/admin/customize/theme-components") + visit("/admin/customize/components") expect(find(".themes-list-header")).to have_css(".components-tab.active") end + + it "switching between themes and components tabs keeps the search visible only if both tabs have at least 10 items" do + 6.times { Fabricate(:theme) } + (1..5).each { |number| Fabricate(:theme, component: true, name: "Cool component #{number}") } + + visit("/admin/customize/themes") + expect(admin_customize_themes_page).to have_themes(count: 11) + + admin_customize_themes_page.search("5") + expect(admin_customize_themes_page).to have_themes(count: 1) + + admin_customize_themes_page.switch_to_components + expect(admin_customize_themes_page).to have_no_search + expect(admin_customize_themes_page).to have_themes(count: 5) + + (6..11).each { |number| Fabricate(:theme, component: true, name: "Cool component #{number}") } + + visit("/admin/customize/components") + expect(admin_customize_themes_page).to have_themes(count: 11) + + admin_customize_themes_page.search("5") + expect(admin_customize_themes_page).to have_themes(count: 1) + + admin_customize_themes_page.switch_to_themes + expect(admin_customize_themes_page).to have_themes(count: 1) + end end describe "when visiting the page to customize a single theme" do diff --git a/spec/system/page_objects/pages/admin_customize_themes.rb b/spec/system/page_objects/pages/admin_customize_themes.rb index 540e1392f34..5c8b005b5c6 100644 --- a/spec/system/page_objects/pages/admin_customize_themes.rb +++ b/spec/system/page_objects/pages/admin_customize_themes.rb @@ -45,6 +45,10 @@ module PageObjects has_css?(".inactive-theme input:checked", count: count) end + def has_themes?(count:) + has_css?(".themes-list-container__item", count: count) + end + def toggle_all_inactive find(".toggle-all-inactive").click end @@ -67,6 +71,22 @@ module PageObjects PageObjects::Components::AdminThemeSettingsEditor.new end + def switch_to_components + find(".components-tab").click + end + + def switch_to_themes + find(".themes-tab").click + end + + def search(term) + find(".themes-list-search__input").fill_in with: term + end + + def has_no_search? + has_no_css?(".themes-list-search__input") + end + private def setting_selector(setting_name, overridden: false)