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.
This commit is contained in:
Alan Guo Xiang Tan 2023-06-28 07:20:31 +08:00 committed by GitHub
parent 4d3999de10
commit aef7c2fe8f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
15 changed files with 28 additions and 410 deletions

View File

@ -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");
}
}

View File

@ -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");
}
}

View File

@ -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", []);
});
}
}

View File

@ -1,52 +1,3 @@
<div
class="control-group preferences-navigation-menu-categories"
data-setting-name="user-navigation-menu-categories"
>
<legend class="control-label">{{i18n
"user.experimental_sidebar.categories_section"
}}</legend>
<div class="controls">
<CategorySelector
@categories={{this.selectedSidebarCategories}}
@onChange={{action (mut this.selectedSidebarCategories)}}
@options={{hash
allowUncategorized=(not this.siteSettings.suppress_uncategorized_badge)
displayCategoryDescription=true
}}
/>
</div>
<div class="instructions">{{i18n
"user.experimental_sidebar.categories_section_instruction"
}}</div>
</div>
{{#if this.model.display_sidebar_tags}}
<div
class="control-group preferences-navigation-menu-tags"
data-setting-name="user-navigation-menu-tags"
>
<legend class="control-label">{{i18n
"user.experimental_sidebar.tags_section"
}}</legend>
<div class="controls">
<TagChooser
@tags={{this.selectedSidebarTagNames}}
@everyTag={{true}}
@unlimitedTagCount={{true}}
@onChange={{action (mut this.selectedSidebarTagNames)}}
@options={{hash allowAny=false}}
/>
</div>
<div class="instructions">{{i18n
"user.experimental_sidebar.tags_section_instruction"
}}</div>
</div>
{{/if}}
<div
class="control-group preferences-navigation-menu-navigation"
data-setting-name="user-navigation-menu-navigation"

View File

@ -116,6 +116,7 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
setupUserSidebarCategories();
await visit("/t/280");
await click(
".sidebar-section[data-section-name='categories'] .sidebar-section-header"
);
@ -132,14 +133,14 @@ acceptance("Sidebar - Logged on user - Categories Section", function (needs) {
setupUserSidebarCategories();
await visit("/");
await click(
".sidebar-section[data-section-name='categories'] .sidebar-section-header-button"
);
assert.strictEqual(
currentURL(),
"/u/eviltrout/preferences/navigation-menu",
"it should transition to user preferences navigation menu page"
assert.true(
exists(".sidebar-categories-form-modal"),
"it shows the categories form modal"
);
});

View File

@ -106,14 +106,14 @@ acceptance("Sidebar - Logged on user - Tags section", function (needs) {
test("clicking on section header button", async function (assert) {
await visit("/");
await click(
".sidebar-section[data-section-name='tags'] .sidebar-section-header-button"
);
assert.strictEqual(
currentURL(),
"/u/eviltrout/preferences/navigation-menu",
"it should transition to user preferences navigation menu page"
assert.true(
exists(".sidebar-tags-form-modal"),
"it shows the tags form modal"
);
});

View File

@ -4,281 +4,13 @@ import { click, visit } from "@ember/test-helpers";
import {
acceptance,
exists,
updateCurrentUser,
} from "discourse/tests/helpers/qunit-helpers";
import selectKit from "discourse/tests/helpers/select-kit-helper";
import Site from "discourse/models/site";
import I18n from "I18n";
acceptance("User Preferences - Navigation Menu", function (needs) {
needs.user({
sidebar_category_ids: [],
sidebar_tags: [],
display_sidebar_tags: true,
});
needs.settings({
navigation_menu: "sidebar",
tagging_enabled: true,
});
let updateUserRequestBody = null;
needs.hooks.afterEach(() => {
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;

View File

@ -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

View File

@ -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

View File

@ -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"

View File

@ -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

View File

@ -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

View File

@ -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"

View File

@ -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"

View File

@ -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",
)