From aef7c2fe8fde5841c34d2c26bb56e0c16c06da57 Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Wed, 28 Jun 2023 07:20:31 +0800 Subject: [PATCH] UX: Use modals to edit categories and tags that appear in sidebar (#22295) Why this change? We are currently not fully satisfied with the current way to edit the categories and tags that appears in the sidebar where the user is redirected to the tracking preferences tab in the user's profile causing the user to lose context of the current page. In addition, the dropdown to select categories or tags limits the amount of information we can display. Since editing or adding a custom categories section is already using a modal, we have decided to switch editing the categories and tags that appear in the sidebar to use a modal as well. This commit removes the `new_edit_sidebar_categories_tags_interface_groups` site setting and make the modals the default for all users. --- .../sidebar/user/categories-section.js | 8 +- .../components/sidebar/user/tags-section.js | 8 +- .../preferences/navigation-menu.js | 22 +- .../templates/preferences/navigation-menu.hbs | 49 ---- .../sidebar-user-categories-section-test.js | 9 +- .../sidebar-user-tags-section-test.js | 8 +- .../user-preferences-navigation-menu-test.js | 270 +----------------- app/models/user.rb | 4 - app/serializers/current_user_serializer.rb | 1 - config/locales/client.en.yml | 4 - config/site_settings.yml | 6 - ...ries_tags_interface_groups_site_setting.rb | 13 + ...ting_sidebar_categories_navigation_spec.rb | 6 +- .../editing_sidebar_tags_navigation_spec.rb | 6 +- ...iewing_navigation_menu_preferences_spec.rb | 24 -- 15 files changed, 28 insertions(+), 410 deletions(-) create mode 100644 db/migrate/20230627060104_remove_new_edit_sidebar_categories_tags_interface_groups_site_setting.rb diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/categories-section.js b/app/assets/javascripts/discourse/app/components/sidebar/user/categories-section.js index 74cbe423404..7865e4d4f14 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/categories-section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/categories-section.js @@ -66,12 +66,6 @@ export default class SidebarUserCategoriesSection extends SidebarCommonCategorie @action editTracked() { - if ( - this.currentUser.new_edit_sidebar_categories_tags_interface_groups_enabled - ) { - showModal("sidebar-categories-form"); - } else { - this.router.transitionTo("preferences.navigation-menu", this.currentUser); - } + showModal("sidebar-categories-form"); } } diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/tags-section.js b/app/assets/javascripts/discourse/app/components/sidebar/user/tags-section.js index 5c3fa3dde08..65d8b107937 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/tags-section.js +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/tags-section.js @@ -80,12 +80,6 @@ export default class SidebarUserTagsSection extends SidebarCommonTagsSection { @action editTracked() { - if ( - this.currentUser.new_edit_sidebar_categories_tags_interface_groups_enabled - ) { - showModal("sidebar-tags-form"); - } else { - this.router.transitionTo("preferences.navigation-menu", this.currentUser); - } + showModal("sidebar-tags-form"); } } diff --git a/app/assets/javascripts/discourse/app/controllers/preferences/navigation-menu.js b/app/assets/javascripts/discourse/app/controllers/preferences/navigation-menu.js index 9ec75d04f30..c6b512189f8 100644 --- a/app/assets/javascripts/discourse/app/controllers/preferences/navigation-menu.js +++ b/app/assets/javascripts/discourse/app/controllers/preferences/navigation-menu.js @@ -7,33 +7,21 @@ import { popupAjaxError } from "discourse/lib/ajax-error"; export default class extends Controller { @tracked saved = false; - @tracked selectedSidebarCategories = []; - @tracked selectedSidebarTagNames = []; subpageTitle = I18n.t("user.preferences_nav.navigation_menu"); saveAttrNames = [ - "sidebar_category_ids", - "sidebar_tag_names", "sidebar_link_to_filtered_list", "sidebar_show_count_of_new_items", ]; @action save() { - const initialSidebarCategoryIds = this.model.sidebarCategoryIds; const initialSidebarLinkToFilteredList = this.model.sidebarLinkToFilteredList; const initialSidebarShowCountOfNewItems = this.model.sidebarShowCountOfNewItems; - this.model.set( - "sidebarCategoryIds", - this.selectedSidebarCategories.mapBy("id") - ); - - this.model.set("sidebar_tag_names", this.selectedSidebarTagNames); - this.model.set( "user_option.sidebar_link_to_filtered_list", this.newSidebarLinkToFilteredList @@ -45,15 +33,10 @@ export default class extends Controller { this.model .save(this.saveAttrNames) - .then((result) => { - if (result.user.sidebar_tags) { - this.model.set("sidebar_tags", result.user.sidebar_tags); - } - + .then(() => { this.saved = true; }) .catch((error) => { - this.model.set("sidebarCategoryIds", initialSidebarCategoryIds); this.model.set( "user_option.sidebar_link_to_filtered_list", initialSidebarLinkToFilteredList @@ -64,9 +47,6 @@ export default class extends Controller { ); popupAjaxError(error); - }) - .finally(() => { - this.model.set("sidebar_tag_names", []); }); } } diff --git a/app/assets/javascripts/discourse/app/templates/preferences/navigation-menu.hbs b/app/assets/javascripts/discourse/app/templates/preferences/navigation-menu.hbs index a1aab208b4d..30ee0bd0a38 100644 --- a/app/assets/javascripts/discourse/app/templates/preferences/navigation-menu.hbs +++ b/app/assets/javascripts/discourse/app/templates/preferences/navigation-menu.hbs @@ -1,52 +1,3 @@ -
- {{i18n - "user.experimental_sidebar.categories_section" - }} - -
- -
- -
{{i18n - "user.experimental_sidebar.categories_section_instruction" - }}
-
- -{{#if this.model.display_sidebar_tags}} -
- {{i18n - "user.experimental_sidebar.tags_section" - }} - -
- -
- -
{{i18n - "user.experimental_sidebar.tags_section_instruction" - }}
-
-{{/if}} -
{ - updateUserRequestBody = null; - }); - - needs.pretender((server, helper) => { - server.put("/u/eviltrout.json", (request) => { - updateUserRequestBody = helper.parsePostData(request.requestBody); - - // if only the howto category is updated, intentionally cause an error - if ( - updateUserRequestBody["sidebar_category_ids[]"]?.[0] === "10" || - updateUserRequestBody["sidebar_tag_names[]"]?.[0] === "gazelle" - ) { - // This request format will cause an error - return helper.response(400, {}); - } else { - return helper.response({ - user: { - sidebar_tags: [ - { name: "monkey", pm_only: false }, - { name: "gazelle", pm_only: false }, - ], - }, - }); - } - }); - }); - - test("sidebar preferences link is not shown when navigation menu is set to legacy", async function (assert) { - this.siteSettings.navigation_menu = "legacy"; - - await visit("/u/eviltrout/preferences"); - - assert.dom(".nav-sidebar").doesNotExist(); - }); - - test("user encountering error when adding categories to sidebar", async function (assert) { - updateCurrentUser({ sidebar_category_ids: [6] }); - - await visit("/"); - - assert.ok( - exists( - ".sidebar-section[data-section-name='categories'] .sidebar-section-link-wrapper[data-category-id=6] a" - ), - "support category is present in sidebar" - ); - - await click( - ".sidebar-section[data-section-name='categories'] .sidebar-section-header-button" - ); - - const categorySelector = selectKit(".category-selector"); - await categorySelector.expand(); - await categorySelector.selectKitSelectRowByName("howto"); - await categorySelector.deselectItemByName("support"); - - await click(".save-changes"); - - assert.deepEqual( - updateUserRequestBody["sidebar_category_ids[]"], - ["10"], - "contains the right request body to update user's sidebar category links" - ); - - assert.ok(exists(".dialog-body"), "error message is displayed"); - - await click(".dialog-footer .btn-primary"); - - assert.ok( - !exists( - ".sidebar-section[data-section-name='categories'] .sidebar-section-link-wrapper[data-category-id=10] a" - ), - "howto category is not displayed in sidebar" - ); - - assert.ok( - exists( - ".sidebar-section[data-section-name='categories'] .sidebar-section-link-wrapper[data-category-id=6] a" - ), - "support category is displayed in sidebar" - ); - }); - - test("user adding categories to sidebar when default sidebar categories have not been configured", async function (assert) { - updateCurrentUser({ admin: false, display_sidebar_tags: false }); - await visit("/u/eviltrout/preferences/navigation-menu"); - - const categorySelector = selectKit(".category-selector"); - await categorySelector.expand(); - await categorySelector.selectKitSelectRowByName("support"); - await categorySelector.selectKitSelectRowByName("bug"); - - await click(".save-changes"); - - assert.ok( - exists( - ".sidebar-section[data-section-name='categories'] .sidebar-section-link-wrapper[data-category-id=6] a" - ), - "support category has been added to sidebar" - ); - - assert.ok( - exists( - ".sidebar-section[data-section-name='categories'] .sidebar-section-link-wrapper[data-category-id=1] a" - ), - "bug category has been added to sidebar" - ); - }); - - test("user adding categories to sidebar when default sidebar categories have been configured", async function (assert) { - this.siteSettings.default_navigation_menu_categories = "5"; - - await visit("/"); - await click( - ".sidebar-section[data-section-name='categories'] .sidebar-section-header-button" - ); - - const categorySelector = selectKit(".category-selector"); - await categorySelector.expand(); - await categorySelector.selectKitSelectRowByName("support"); - await categorySelector.selectKitSelectRowByName("bug"); - - await click(".save-changes"); - - assert.ok( - exists( - ".sidebar-section[data-section-name='categories'] .sidebar-section-link-wrapper[data-category-id=6] a" - ), - "support category has been added to sidebar" - ); - - assert.ok( - exists( - ".sidebar-section[data-section-name='categories'] .sidebar-section-link-wrapper[data-category-id=1] a" - ), - "bug category has been added to sidebar" - ); - - assert.deepEqual( - updateUserRequestBody["sidebar_category_ids[]"], - ["6", "1"], - "contains the right request body to update user's sidebar category links" - ); - }); - - test("user encountering error when adding tags to sidebar", async function (assert) { - updateCurrentUser({ sidebar_tags: [{ name: "monkey", pm_only: false }] }); - - await visit("/"); - - assert.ok( - exists( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name=monkey]" - ), - "monkey tag is displayed in sidebar" - ); - - await click( - ".sidebar-section[data-section-name='tags'] .sidebar-section-header-button" - ); - - const tagChooser = selectKit(".tag-chooser"); - await tagChooser.expand(); - await tagChooser.selectKitSelectRowByName("gazelle"); - await tagChooser.deselectItemByName("monkey"); - - await click(".save-changes"); - - assert.deepEqual( - updateUserRequestBody["sidebar_tag_names[]"], - ["gazelle"], - "contains the right request body to update user's sidebar tag links" - ); - - assert.ok(exists(".dialog-body"), "error message is displayed"); - - await click(".dialog-footer .btn-primary"); - - assert.ok( - !exists( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name=gazelle]" - ), - "gazelle tag is not displayed in sidebar" - ); - - assert.ok( - exists( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name=monkey]" - ), - "monkey tag is displayed in sidebar" - ); - }); - - test("user should not see tag chooser when display_sidebar_tags property is false", async function (assert) { - updateCurrentUser({ display_sidebar_tags: false }); - - await visit("/u/eviltrout/preferences/navigation-menu"); - - assert.ok(!exists(".tag-chooser"), "tag chooser is not displayed"); - }); - - test("user adding tags to sidebar when default tags have not been configured", async function (assert) { - await visit("/u/eviltrout/preferences/navigation-menu"); - - const tagChooser = selectKit(".tag-chooser"); - await tagChooser.expand(); - await tagChooser.selectKitSelectRowByName("monkey"); - - await click(".save-changes"); - - assert.ok( - exists( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name=monkey]" - ), - "monkey tag has been added to sidebar" - ); - }); - - test("user adding tags to sidebar when default tags have been configured", async function (assert) { - this.siteSettings.default_navigation_menu_tags = "tag1|tag2"; - - await visit("/"); - await click( - ".sidebar-section[data-section-name='tags'] .sidebar-section-header-button" - ); - - const tagChooser = selectKit(".tag-chooser"); - await tagChooser.expand(); - await tagChooser.selectKitSelectRowByName("monkey"); - await tagChooser.selectKitSelectRowByName("gazelle"); - - await click(".save-changes"); - - assert.ok( - exists( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name=monkey]" - ), - "monkey tag has been added to sidebar" - ); - - assert.ok( - exists( - ".sidebar-section[data-section-name='tags'] .sidebar-section-link-wrapper[data-tag-name=gazelle]" - ), - "gazelle tag has been added to sidebar" - ); - - assert.deepEqual( - updateUserRequestBody["sidebar_tag_names[]"], - ["monkey", "gazelle"], - "contains the right request body to update user's sidebar tag links" - ); - }); + needs.user(); test("user enabling sidebar_show_count_of_new_items preference", async function (assert) { const categories = Site.current().categories; diff --git a/app/models/user.rb b/app/models/user.rb index 8e0e0dbc4eb..04010c61f4a 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -1807,10 +1807,6 @@ class User < ActiveRecord::Base in_any_groups?(SiteSetting.experimental_new_new_view_groups_map) end - def new_edit_sidebar_categories_tags_interface_groups_enabled? - in_any_groups?(SiteSetting.new_edit_sidebar_categories_tags_interface_groups_map) - end - def experimental_search_menu_groups_enabled? in_any_groups?(SiteSetting.experimental_search_menu_groups_map) end diff --git a/app/serializers/current_user_serializer.rb b/app/serializers/current_user_serializer.rb index 7290c5c5f38..0a8e5c2577f 100644 --- a/app/serializers/current_user_serializer.rb +++ b/app/serializers/current_user_serializer.rb @@ -68,7 +68,6 @@ class CurrentUserSerializer < BasicUserSerializer :sidebar_category_ids, :sidebar_sections, :new_new_view_enabled?, - :new_edit_sidebar_categories_tags_interface_groups_enabled?, :experimental_search_menu_groups_enabled? delegate :user_stat, to: :object, private: true diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index 82ba0c212f3..20d92db0a39 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1206,10 +1206,6 @@ en: experimental_sidebar: enable: "Enable sidebar" options: "Options" - categories_section: "Categories Section" - categories_section_instruction: "Selected categories will be displayed under Navigation Menu's categories section. If no categories are selected, the site's top categories will be displayed." - tags_section: "Tags Section" - tags_section_instruction: "Selected tags will be displayed under Navigation Menu's tags section. If no tags are selected, the site's top tags will be displayed." navigation_section: "Navigation" navigation_section_instruction: "When a topic list in the navigation menu has new or unread items…" link_to_filtered_list_checkbox_description: "Link to the filtered list" diff --git a/config/site_settings.yml b/config/site_settings.yml index 6fb199d151d..da9c1e27c3e 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -2112,12 +2112,6 @@ developer: experimental_topics_filter: client: true default: false - new_edit_sidebar_categories_tags_interface_groups: - type: group_list - list_type: compact - default: "" - allow_any: false - hidden: true experimental_search_menu_groups: type: group_list list_type: compact diff --git a/db/migrate/20230627060104_remove_new_edit_sidebar_categories_tags_interface_groups_site_setting.rb b/db/migrate/20230627060104_remove_new_edit_sidebar_categories_tags_interface_groups_site_setting.rb new file mode 100644 index 00000000000..c933c3e6fc9 --- /dev/null +++ b/db/migrate/20230627060104_remove_new_edit_sidebar_categories_tags_interface_groups_site_setting.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class RemoveNewEditSidebarCategoriesTagsInterfaceGroupsSiteSetting < ActiveRecord::Migration[7.0] + def up + execute( + "DELETE FROM site_settings WHERE name = 'new_edit_sidebar_categories_tags_interface_groups'", + ) + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/spec/system/editing_sidebar_categories_navigation_spec.rb b/spec/system/editing_sidebar_categories_navigation_spec.rb index d1fff77efa0..48ecdda446c 100644 --- a/spec/system/editing_sidebar_categories_navigation_spec.rb +++ b/spec/system/editing_sidebar_categories_navigation_spec.rb @@ -2,7 +2,6 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do fab!(:user) { Fabricate(:user) } - fab!(:group) { Fabricate(:group).tap { |g| g.add(user) } } fab!(:category2) { Fabricate(:category, name: "category2") } @@ -22,10 +21,7 @@ RSpec.describe "Editing sidebar categories navigation", type: :system do let(:sidebar) { PageObjects::Components::Sidebar.new } - before do - SiteSetting.new_edit_sidebar_categories_tags_interface_groups = group.name - sign_in(user) - end + before { sign_in(user) } it "allows a user to edit the sidebar categories navigation" do visit "/latest" diff --git a/spec/system/editing_sidebar_tags_navigation_spec.rb b/spec/system/editing_sidebar_tags_navigation_spec.rb index 71c35fa47e8..2dedc345ba6 100644 --- a/spec/system/editing_sidebar_tags_navigation_spec.rb +++ b/spec/system/editing_sidebar_tags_navigation_spec.rb @@ -2,7 +2,6 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do fab!(:user) { Fabricate(:user) } - fab!(:group) { Fabricate(:group).tap { |g| g.add(user) } } fab!(:tag1) { Fabricate(:tag, name: "tag").tap { |tag| Fabricate.times(3, :topic, tags: [tag]) } } fab!(:tag2) do @@ -18,10 +17,7 @@ RSpec.describe "Editing sidebar tags navigation", type: :system do let(:sidebar) { PageObjects::Components::Sidebar.new } - before do - SiteSetting.new_edit_sidebar_categories_tags_interface_groups = group.name - sign_in(user) - end + before { sign_in(user) } it "allows a user to edit the sidebar categories navigation" do visit "/latest" diff --git a/spec/system/viewing_navigation_menu_preferences_spec.rb b/spec/system/viewing_navigation_menu_preferences_spec.rb index 0fe2220c16a..b8acc1a3ce9 100644 --- a/spec/system/viewing_navigation_menu_preferences_spec.rb +++ b/spec/system/viewing_navigation_menu_preferences_spec.rb @@ -10,22 +10,6 @@ describe "Viewing sidebar preferences", type: :system do context "as an admin" do fab!(:admin) { Fabricate(:admin) } fab!(:user) { Fabricate(:user) } - fab!(:category) { Fabricate(:category) } - fab!(:category2) { Fabricate(:category) } - fab!(:category_sidebar_section_link) do - Fabricate(:category_sidebar_section_link, user: user, linkable: category) - end - fab!(:category2_sidebar_section_link) do - Fabricate(:category_sidebar_section_link, user: user, linkable: category2) - end - fab!(:tag) { Fabricate(:tag) } - fab!(:tag2) { Fabricate(:tag) } - fab!(:tag_sidebar_section_link) do - Fabricate(:tag_sidebar_section_link, user: user, linkable: tag) - end - fab!(:tag2_sidebar_section_link) do - Fabricate(:tag_sidebar_section_link, user: user, linkable: tag2) - end before { sign_in(admin) } @@ -37,14 +21,6 @@ describe "Viewing sidebar preferences", type: :system do user_preferences_navigation_menu_page.visit(user) - expect(user_preferences_navigation_menu_page).to have_navigation_menu_categories_preference( - category, - category2, - ) - expect(user_preferences_navigation_menu_page).to have_navigation_menu_tags_preference( - tag, - tag2, - ) expect(user_preferences_navigation_menu_page).to have_navigation_menu_preference_checked( "pref-show-count-new-items", )