From 07424b9bb63e4044aabe345a09830e4abe8a373c Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Mon, 25 Jul 2022 14:45:16 +0800 Subject: [PATCH] UX: Docking/undocking sidebar toggles sidebar hamburger dropdown (#17636) Before this change, undocking the sidebar would just hide the sidebar from the screen which led people to complain that they "lost" their sidebar and had to "find" it. With this change, we automatically display the sidebar hamburger dropdown when you undock the sidebar. Like wise when the sidebar is docked, the sidebar hamburger dropdown is automatically collapsed. --- .../discourse/app/components/site-header.js | 22 ------- .../discourse/app/controllers/application.js | 7 +- .../templates/components/sidebar/footer.hbs | 2 +- .../components/sidebar/hamburger-dropdown.hbs | 2 +- .../discourse/app/widgets/header.js | 66 +++++++++---------- .../sidebar-categories-section-test.js | 6 +- .../sidebar-community-section-test.js | 6 +- .../acceptance/sidebar-tags-section-test.js | 6 +- .../tests/acceptance/sidebar-test.js | 21 +++++- 9 files changed, 67 insertions(+), 71 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index 9f3c000630..bf24ac13a6 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/app/assets/javascripts/discourse/app/components/site-header.js @@ -236,17 +236,6 @@ const SiteHeaderComponent = MountWidget.extend( this.appEvents.on("user-menu:rendered", this, "_animateMenu"); } - if ( - this.currentUser?.experimental_sidebar_enabled && - !this.site.mobileView - ) { - this.appEvents.on( - "sidebar:docked-state-updated", - this, - "queueRerender" - ); - } - this.dispatch("notifications:changed", "user-notifications"); this.dispatch("header:keyboard-trigger", "header"); this.dispatch("user-menu:navigation", "user-menu"); @@ -363,17 +352,6 @@ const SiteHeaderComponent = MountWidget.extend( this.appEvents.off("user-menu:rendered", this, "_animateMenu"); } - if ( - this.currentUser?.experimental_sidebar_enabled && - !this.site.mobileView - ) { - this.appEvents.off( - "sidebar:docked-state-updated", - this, - "queueRerender" - ); - } - if (this.currentUser) { this.currentUser.off("status-changed", this, "queueRerender"); } diff --git a/app/assets/javascripts/discourse/app/controllers/application.js b/app/assets/javascripts/discourse/app/controllers/application.js index 2e0bfcf816..891e011a2b 100644 --- a/app/assets/javascripts/discourse/app/controllers/application.js +++ b/app/assets/javascripts/discourse/app/controllers/application.js @@ -50,9 +50,12 @@ export default Controller.extend({ discourseDebounce(this, this._mainOutletAnimate, 250); this.toggleProperty("showSidebar"); - this.appEvents.trigger("sidebar:docked-state-updated"); - if (!this.site.mobileView) { + if (this.site.desktopView) { + this.appEvents.trigger("header:keyboard-trigger", { + type: "hamburger", + }); + if (this.showSidebar) { this.keyValueStore.removeItem(this.hideSidebarKey); } else { diff --git a/app/assets/javascripts/discourse/app/templates/components/sidebar/footer.hbs b/app/assets/javascripts/discourse/app/templates/components/sidebar/footer.hbs index 62ce2ab0fb..38fdaedb00 100644 --- a/app/assets/javascripts/discourse/app/templates/components/sidebar/footer.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/sidebar/footer.hbs @@ -19,7 +19,7 @@ @icon="keyboard" @class="sidebar-footer-actions-button sidebar-footer-actions-keyboard-shortcuts" /> - {{#if this.site.desktopView}} + {{#if (and this.site.desktopView (not @sidebarDocked))}}
- +
diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index dc5b9a86b5..b7988ffc8c 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -244,34 +244,32 @@ createWidget("header-icons", { icons.push(search); - if (!attrs.sidebarDocked) { - const hamburger = this.attach("header-dropdown", { - title: "hamburger_menu", - icon: "bars", - iconId: "toggle-hamburger-menu", - active: attrs.hamburgerVisible, - action: "toggleHamburger", - href: "", - classNames: ["hamburger-dropdown"], + const hamburger = this.attach("header-dropdown", { + title: "hamburger_menu", + icon: "bars", + iconId: "toggle-hamburger-menu", + active: attrs.hamburgerVisible, + action: "toggleHamburger", + href: "", + classNames: ["hamburger-dropdown"], - contents() { - let { currentUser } = this; - if (currentUser && currentUser.reviewable_count) { - return h( - "div.badge-notification.reviewables", - { - attributes: { - title: I18n.t("notifications.reviewable_items"), - }, + contents() { + let { currentUser } = this; + if (currentUser && currentUser.reviewable_count) { + return h( + "div.badge-notification.reviewables", + { + attributes: { + title: I18n.t("notifications.reviewable_items"), }, - this.currentUser.reviewable_count - ); - } - }, - }); + }, + this.currentUser.reviewable_count + ); + } + }, + }); - icons.push(hamburger); - } + icons.push(hamburger); if (attrs.user) { icons.push( @@ -339,12 +337,15 @@ createWidget("revamped-hamburger-menu-wrapper", { return { "data-click-outside": true }; }, - html() { + html(attrs) { return [ new RenderGlimmer( this, "div.widget-component-connector", - hbs`` + hbs``, + { + sidebarDocked: attrs.sidebarDocked, + } ), ]; }, @@ -402,10 +403,6 @@ export default createWidget("header", { inTopicRoute = this.router.currentRouteName.startsWith("topic."); } - if (attrs.sidebarDocked) { - state.hamburgerVisible = false; - } - let contents = () => { const headerIcons = this.attach("header-icons", { hamburgerVisible: state.hamburgerVisible, @@ -413,7 +410,6 @@ export default createWidget("header", { searchVisible: state.searchVisible, ringBackdrop: state.ringBackdrop, flagCount: attrs.flagCount, - sidebarDocked: attrs.sidebarDocked, user: this.currentUser, }); @@ -431,7 +427,11 @@ export default createWidget("header", { ); } else if (state.hamburgerVisible) { if (this.currentUser?.experimental_sidebar_enabled) { - panels.push(this.attach("revamped-hamburger-menu-wrapper", {})); + panels.push( + this.attach("revamped-hamburger-menu-wrapper", { + sidebarDocked: attrs.sidebarDocked, + }) + ); } else { panels.push(this.attach("hamburger-menu")); } diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js index 0a32bdb29a..4404085db4 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-categories-section-test.js @@ -399,9 +399,9 @@ acceptance("Sidebar - Categories Section", function (needs) { await undockSidebar(); - assert.ok( - Object.keys(topicTrackingState.stateChangeCallbacks).length < - initialCallbackCount + assert.strictEqual( + Object.keys(topicTrackingState.stateChangeCallbacks).length, + initialCallbackCount ); }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js index 732055afaf..5e62985755 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-community-section-test.js @@ -755,9 +755,9 @@ acceptance("Sidebar - Community Section", function (needs) { await undockSidebar(); - assert.ok( - Object.keys(topicTrackingState.stateChangeCallbacks).length < - initialCallbackCount + assert.strictEqual( + Object.keys(topicTrackingState.stateChangeCallbacks).length, + initialCallbackCount ); }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js index fd462bef42..a6cbd2182e 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-tags-section-test.js @@ -326,9 +326,9 @@ acceptance("Sidebar - Tags section", function (needs) { await undockSidebar(); - assert.ok( - Object.keys(topicTrackingState.stateChangeCallbacks).length < - initialCallbackCount + assert.strictEqual( + Object.keys(topicTrackingState.stateChangeCallbacks).length, + initialCallbackCount ); }); }); diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js index 93179c42ef..aebb1f1fe3 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-test.js @@ -90,11 +90,9 @@ acceptance("Sidebar - User with sidebar enabled", function (needs) { assert.ok(!exists(".sidebar-container"), "hides the sidebar"); - await click(".hamburger-dropdown"); - assert.ok( exists(".sidebar-hamburger-dropdown"), - "displays the sidebar in hamburger dropdown" + "displays the sidebar in hamburger dropdown automatically after undocking" ); await click("button.sidebar-footer-actions-dock-toggle"); @@ -103,5 +101,22 @@ acceptance("Sidebar - User with sidebar enabled", function (needs) { exists(".sidebar-container"), "displays the sidebar after docking" ); + + assert.notOk( + exists(".sidebar-hamburger-dropdown"), + "hides the sidebar in hamburger dropdown automatically after docking" + ); + + await click(".hamburger-dropdown"); + + assert.ok( + exists(".sidebar-hamburger-dropdown"), + "displays the sidebar in hamburger dropdown even when sidebar is docked" + ); + + assert.notOk( + exists(".sidebar-hamburger-dropdown .sidebar-footer-actions-dock-toggle"), + "does not display sidebar dock toggle in hamburger dropdown when sidebar is docked" + ); }); });