From 4f7f9ef87cbcd574144f657dd43b7e705d98ff8e Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 27 Jun 2023 10:31:48 +0800 Subject: [PATCH] UX: Order categories in edit navigation menu modal by name (#22291) Why does this change do? If the `fixed_category_positions` is `false`, we want to order the categories in the edit navigation menu categories modal by name. This makes it easier to filter through a large list of categories. This commit also fixes a bug where we were unintentionally mutating the `this.site.categories` array. --- .../sidebar/common/categories-section.js | 4 +-- .../categories-form.js | 10 ++++++- ...ting_sidebar_categories_navigation_spec.rb | 30 ++++++++++++++----- .../modals/sidebar_edit_categories.rb | 16 +++++----- 4 files changed, 41 insertions(+), 19 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/sidebar/common/categories-section.js b/app/assets/javascripts/discourse/app/components/sidebar/common/categories-section.js index dd45f8d2228..374f03f8b0a 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/common/categories-section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/common/categories-section.js @@ -38,10 +38,10 @@ export default class SidebarCommonCategoriesSection extends Component { return this.categories; } - let categories = this.site.categories; + let categories = [...this.site.categories]; if (!this.siteSettings.fixed_category_positions) { - categories = categories.sort((a, b) => a.name.localeCompare(b.name)); + categories.sort((a, b) => a.name.localeCompare(b.name)); } const categoryIds = this.categories.map((category) => category.id); diff --git a/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/categories-form.js b/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/categories-form.js index a856ba10b23..a515bf6184a 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/categories-form.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/edit-navigation-modal-form/categories-form.js @@ -6,6 +6,7 @@ import { tracked } from "@glimmer/tracking"; import { INPUT_DELAY } from "discourse-common/config/environment"; import discourseDebounce from "discourse-common/lib/debounce"; import { popupAjaxError } from "discourse/lib/ajax-error"; +import Category from "discourse/models/category"; export default class extends Component { @service currentUser; @@ -26,7 +27,13 @@ export default class extends Component { constructor() { super(...arguments); - this.site.sortedCategories.reduce( + let categories = [...this.site.categories]; + + if (!this.siteSettings.fixed_category_positions) { + categories.sort((a, b) => a.name.localeCompare(b.name)); + } + + Category.sortCategories(categories).reduce( (categoryGrouping, category, index, arr) => { if (category.isUncategorizedCategory) { return categoryGrouping; @@ -35,6 +42,7 @@ export default class extends Component { categoryGrouping.push(category); const nextCategory = arr[index + 1]; + if (!nextCategory || nextCategory.level === 0) { this.categoryGroupings.push(categoryGrouping); return []; diff --git a/spec/system/editing_sidebar_categories_navigation_spec.rb b/spec/system/editing_sidebar_categories_navigation_spec.rb index 3140b327f1a..d1fff77efa0 100644 --- a/spec/system/editing_sidebar_categories_navigation_spec.rb +++ b/spec/system/editing_sidebar_categories_navigation_spec.rb @@ -3,14 +3,6 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do fab!(:user) { Fabricate(:user) } fab!(:group) { Fabricate(:group).tap { |g| g.add(user) } } - fab!(:category) { Fabricate(:category, name: "category") } - fab!(:category_subcategory) do - Fabricate(:category, parent_category_id: category.id, name: "category subcategory") - end - - fab!(:category_subcategory2) do - Fabricate(:category, parent_category_id: category.id, name: "category subcategory 2") - end fab!(:category2) { Fabricate(:category, name: "category2") } @@ -18,6 +10,16 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do Fabricate(:category, parent_category_id: category2.id, name: "category2 subcategory") end + fab!(:category) { Fabricate(:category, name: "category") } + + fab!(:category_subcategory2) do + Fabricate(:category, parent_category_id: category.id, name: "category subcategory 2") + end + + fab!(:category_subcategory) do + Fabricate(:category, parent_category_id: category.id, name: "category subcategory") + end + let(:sidebar) { PageObjects::Components::Sidebar.new } before do @@ -72,6 +74,18 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do expect(sidebar).to have_no_section_link(category2.name) end + it "displays the categories in the modal based on the fixed position of the category when `fixed_category_positions` site setting is enabled" do + SiteSetting.fixed_category_positions = true + + visit "/latest" + + modal = sidebar.click_edit_categories_button + + expect(modal).to have_categories( + [category2, category2_subcategory, category, category_subcategory2, category_subcategory], + ) + end + it "allows a user to deselect all categories in the modal" do Fabricate(:category_sidebar_section_link, linkable: category, user: user) Fabricate(:category_sidebar_section_link, linkable: category_subcategory2, user: user) diff --git a/spec/system/page_objects/modals/sidebar_edit_categories.rb b/spec/system/page_objects/modals/sidebar_edit_categories.rb index 4e78c000d53..187f60ae575 100644 --- a/spec/system/page_objects/modals/sidebar_edit_categories.rb +++ b/spec/system/page_objects/modals/sidebar_edit_categories.rb @@ -36,15 +36,15 @@ module PageObjects end def has_categories?(categories) - category_ids = categories.map(&:id) + category_names = categories.map(&:name) - has_css?( - ".sidebar-categories-form-modal .sidebar-categories-form__category-row", - count: category_ids.length, - ) && - all(".sidebar-categories-form-modal .sidebar-categories-form__category-row").all? do |row| - category_ids.include?(row["data-category-id"].to_i) - end + categories = + all( + ".sidebar-categories-form-modal .sidebar-categories-form__category-row", + count: category_names.length, + ) + + expect(categories.map(&:text)).to eq(category_names) end def has_checkbox?(category, disabled: false)