From 7a05a9d4110cd3ad8c085750b9e2eb6590aeb75e Mon Sep 17 00:00:00 2001 From: Alan Guo Xiang Tan Date: Tue, 16 Aug 2022 13:45:32 +0800 Subject: [PATCH] UX: Make Sidebar more consistent with user menu on mobile (#17940) Before this commit, we carried custom code and styles for the sidebar on mobile. This meant the look and feel of bringing up the sidebar on mobile was very different from the user menu resulting in a very inconsistent experience on mobile. Also, we could not leverage on the existing swipe to close support on mobile. In this commit, we made it such that the sidebar dropdown is always rendered on mobile and made the interaction with the dropdown more consistent with the user menu. There is also more parity with the old hamburger dropdown when the experimental sidebar is disabled. --- .../components/sidebar/hamburger-dropdown.js | 12 +++++ .../discourse/app/components/site-header.js | 23 +++++++++ .../discourse/app/controllers/application.js | 5 ++ .../components/sidebar/hamburger-dropdown.hbs | 4 +- .../discourse/app/widgets/header.js | 2 +- .../tests/acceptance/sidebar-mobile-test.js | 30 +++-------- .../stylesheets/common/base/menu-panel.scss | 28 ---------- .../common/base/sidebar-footer.scss | 2 +- .../base/sidebar-more-section-links.scss | 8 ++- .../stylesheets/desktop/menu-panel.scss | 28 ++++++++++ app/assets/stylesheets/mobile/discourse.scss | 51 ------------------- 11 files changed, 86 insertions(+), 107 deletions(-) create mode 100644 app/assets/javascripts/discourse/app/components/sidebar/hamburger-dropdown.js diff --git a/app/assets/javascripts/discourse/app/components/sidebar/hamburger-dropdown.js b/app/assets/javascripts/discourse/app/components/sidebar/hamburger-dropdown.js new file mode 100644 index 00000000000..d2fd84a4384 --- /dev/null +++ b/app/assets/javascripts/discourse/app/components/sidebar/hamburger-dropdown.js @@ -0,0 +1,12 @@ +import Component from "@glimmer/component"; +import { action } from "@ember/object"; +import { inject as service } from "@ember/service"; + +export default class SidebarHamburgerDropdown extends Component { + @service appEvents; + + @action + triggerRenderedAppEvent() { + this.appEvents.trigger("sidebar-hamburger-dropdown:rendered"); + } +} diff --git a/app/assets/javascripts/discourse/app/components/site-header.js b/app/assets/javascripts/discourse/app/components/site-header.js index b5a4dc205dc..888c050fda0 100644 --- a/app/assets/javascripts/discourse/app/components/site-header.js +++ b/app/assets/javascripts/discourse/app/components/site-header.js @@ -225,6 +225,17 @@ const SiteHeaderComponent = MountWidget.extend( this.appEvents.on("user-menu:rendered", this, "_animateMenu"); } + if ( + this.siteSettings.enable_experimental_sidebar_hamburger && + !this.sidebarEnabled + ) { + this.appEvents.on( + "sidebar-hamburger-dropdown:rendered", + this, + "_animateMenu" + ); + } + this.dispatch("notifications:changed", "user-notifications"); this.dispatch("header:keyboard-trigger", "header"); this.dispatch("user-menu:navigation", "user-menu"); @@ -341,6 +352,17 @@ const SiteHeaderComponent = MountWidget.extend( this.appEvents.off("user-menu:rendered", this, "_animateMenu"); } + if ( + this.siteSettings.enable_experimental_sidebar_hamburger && + !this.sidebarEnabled + ) { + this.appEvents.off( + "sidebar-hamburger-dropdown:rendered", + this, + "_animateMenu" + ); + } + if (this.currentUser) { this.currentUser.off("status-changed", this, "queueRerender"); } @@ -373,6 +395,7 @@ const SiteHeaderComponent = MountWidget.extend( _animateMenu() { const menuPanels = document.querySelectorAll(".menu-panel"); + if (menuPanels.length === 0) { if (this.site.mobileView) { this._animate = true; diff --git a/app/assets/javascripts/discourse/app/controllers/application.js b/app/assets/javascripts/discourse/app/controllers/application.js index b91d46572f1..b6d0f25c8bd 100644 --- a/app/assets/javascripts/discourse/app/controllers/application.js +++ b/app/assets/javascripts/discourse/app/controllers/application.js @@ -74,6 +74,11 @@ export default Controller.extend({ return false; } + // Always return dropdown on mobile + if (this.site.mobileView) { + return false; + } + return enableSidebar; }, diff --git a/app/assets/javascripts/discourse/app/templates/components/sidebar/hamburger-dropdown.hbs b/app/assets/javascripts/discourse/app/templates/components/sidebar/hamburger-dropdown.hbs index 3167355bc97..437f5ba090a 100644 --- a/app/assets/javascripts/discourse/app/templates/components/sidebar/hamburger-dropdown.hbs +++ b/app/assets/javascripts/discourse/app/templates/components/sidebar/hamburger-dropdown.hbs @@ -1,7 +1,7 @@
-
+
-
+
diff --git a/app/assets/javascripts/discourse/app/widgets/header.js b/app/assets/javascripts/discourse/app/widgets/header.js index 8fef0b8e65d..56ebb4238b0 100644 --- a/app/assets/javascripts/discourse/app/widgets/header.js +++ b/app/assets/javascripts/discourse/app/widgets/header.js @@ -589,7 +589,7 @@ export default createWidget("header", { toggleHamburger() { if ( this.siteSettings.enable_experimental_sidebar_hamburger && - (this.attrs.sidebarEnabled || this.site.mobileView) + this.attrs.sidebarEnabled ) { this.sendWidgetAction("toggleSidebar"); } else { diff --git a/app/assets/javascripts/discourse/tests/acceptance/sidebar-mobile-test.js b/app/assets/javascripts/discourse/tests/acceptance/sidebar-mobile-test.js index 74970e25105..b384c385dab 100644 --- a/app/assets/javascripts/discourse/tests/acceptance/sidebar-mobile-test.js +++ b/app/assets/javascripts/discourse/tests/acceptance/sidebar-mobile-test.js @@ -18,7 +18,10 @@ acceptance("Sidebar - Mobile - User with sidebar enabled", function (needs) { test("hidden by default", async function (assert) { await visit("/"); - assert.ok(!exists(".sidebar-container"), "sidebar is not displayed"); + assert.ok( + !exists(".sidebar-hamburger-dropdown"), + "sidebar is not displayed" + ); }); test("clicking outside sidebar collapses it", async function (assert) { @@ -26,11 +29,11 @@ acceptance("Sidebar - Mobile - User with sidebar enabled", function (needs) { await click(".hamburger-dropdown"); - assert.ok(exists(".sidebar-container"), "sidebar is displayed"); + assert.ok(exists(".sidebar-hamburger-dropdown"), "sidebar is displayed"); await click("#main-outlet"); - assert.ok(!exists(".sidebar-container"), "sidebar is collapsed"); + assert.ok(!exists(".sidebar-hamburger-dropdown"), "sidebar is collapsed"); }); test("clicking on a link or button in sidebar collapses it", async function (assert) { @@ -40,7 +43,7 @@ acceptance("Sidebar - Mobile - User with sidebar enabled", function (needs) { await click(".sidebar-section-link-tracked"); assert.ok( - !exists(".sidebar-container"), + !exists(".sidebar-hamburger-dropdown"), "sidebar is collapsed when a button in sidebar is clicked" ); @@ -48,28 +51,11 @@ acceptance("Sidebar - Mobile - User with sidebar enabled", function (needs) { await click(".sidebar-section-header-link"); assert.ok( - !exists(".sidebar-container"), + !exists(".sidebar-hamburger-dropdown"), "sidebar is collapsed when a link in sidebar is clicked" ); }); - test("collapsing sidebar sections does not collapse sidebar", async function (assert) { - await visit("/"); - - await click(".hamburger-dropdown"); - await click(".sidebar-section-header-caret"); - - assert.ok( - !exists(".sidebar-section-community .sidebar-section-content"), - "topics section is collapsed" - ); - - assert.ok( - exists(".sidebar-container"), - "sidebar is not collapsed when clicking on caret to collapse a section in sidebar" - ); - }); - test("button to toggle between mobile and desktop view", async function (assert) { await visit("/"); await click(".hamburger-dropdown"); diff --git a/app/assets/stylesheets/common/base/menu-panel.scss b/app/assets/stylesheets/common/base/menu-panel.scss index 3eb492cf569..73ab7444280 100644 --- a/app/assets/stylesheets/common/base/menu-panel.scss +++ b/app/assets/stylesheets/common/base/menu-panel.scss @@ -601,31 +601,3 @@ div.menu-links-header { color: var(--primary-med-or-secondary-med); } } - -// Sidebar-hamburger hybrid - -.hamburger-menu.revamped { - --d-sidebar-highlight-color: var(--highlight-medium); - width: var(--d-sidebar-width); - - .panel-body-content { - width: 100%; - min-width: 0; // prevent content blowing out width - } - - .sidebar-section-wrapper { - .sidebar-section-header-button { - opacity: 1; - } - - .sidebar-section-link.active { - font-weight: normal; - color: var(--primary-high); - background: var(--tertiary-low); - } - } - - .sidebar-footer-wrapper { - padding: 0.5em 0 0; - } -} diff --git a/app/assets/stylesheets/common/base/sidebar-footer.scss b/app/assets/stylesheets/common/base/sidebar-footer.scss index 773a2c688fb..bd6fa717a77 100644 --- a/app/assets/stylesheets/common/base/sidebar-footer.scss +++ b/app/assets/stylesheets/common/base/sidebar-footer.scss @@ -80,7 +80,7 @@ } } -.hamburger-menu.revamped { +.hamburger-panel .revamped { .sidebar-footer-wrapper { background: var(--secondary); .sidebar-footer-container { diff --git a/app/assets/stylesheets/common/base/sidebar-more-section-links.scss b/app/assets/stylesheets/common/base/sidebar-more-section-links.scss index 5585da35da2..6a5bb281a33 100644 --- a/app/assets/stylesheets/common/base/sidebar-more-section-links.scss +++ b/app/assets/stylesheets/common/base/sidebar-more-section-links.scss @@ -1,6 +1,4 @@ .sidebar-more-section-links-details { - margin-left: 1.5em; - .sidebar-more-section-links-details-summary { padding: 0.35em 0.5em; color: var(--primary-high); @@ -41,3 +39,9 @@ border-top: 1.5px solid var(--primary-low); } } + +#main-outlet-wrapper { + .sidebar-more-section-links-details { + margin-left: 1.5em; + } +} diff --git a/app/assets/stylesheets/desktop/menu-panel.scss b/app/assets/stylesheets/desktop/menu-panel.scss index f70261e4018..baa7227df89 100644 --- a/app/assets/stylesheets/desktop/menu-panel.scss +++ b/app/assets/stylesheets/desktop/menu-panel.scss @@ -7,3 +7,31 @@ max-height: calc(100vh - 100px); } } + +// Sidebar-hamburger hybrid + +.hamburger-panel .revamped { + --d-sidebar-highlight-color: var(--highlight-medium); + width: var(--d-sidebar-width); + + .panel-body-content { + width: 100%; + min-width: 0; // prevent content blowing out width + } + + .sidebar-section-wrapper { + .sidebar-section-header-button { + opacity: 1; + } + + .sidebar-section-link.active { + font-weight: normal; + color: var(--primary-high); + background: var(--tertiary-low); + } + } + + .sidebar-footer-wrapper { + padding: 0.5em 0 0; + } +} diff --git a/app/assets/stylesheets/mobile/discourse.scss b/app/assets/stylesheets/mobile/discourse.scss index 1af398ad3db..ec241e17f3c 100644 --- a/app/assets/stylesheets/mobile/discourse.scss +++ b/app/assets/stylesheets/mobile/discourse.scss @@ -140,54 +140,3 @@ blockquote { #simple-container { width: 90%; } - -// Sidebar styles - -#main-outlet-wrapper { - grid-template-columns: minmax(0, 100vw); - grid-template-areas: "content"; - gap: 0; - - .sidebar-wrapper { - width: 0; - transition: width 0.2s ease-in-out; - z-index: z("modal", "content"); - grid-area: content; - justify-self: end; - } - - .sidebar-container { - padding-bottom: 6.6em; // extra space to watch out for navbar - } -} - -body.has-sidebar-page { - .d-header-wrap { - // edits the z-index when the sidebar is open: - // header - // sidebar & body overlay - // composer - z-index: z("modal", "content") + 1; - } - - #main-outlet-wrapper { - .sidebar-wrapper { - width: var(--d-sidebar-width); - margin-right: -10px; // compensate for main-outlet-wrapper padding - } - } - - #main-outlet { - &:after { - content: ""; - background: rgb(0, 0, 0); // always a black overlay - position: absolute; - top: 0; - left: 0; - right: 0; - bottom: 0; - z-index: z("modal", "overlay"); - opacity: 0.5; - } - } -}