From 89ff7d51e6b49e7046cd06b0e6f6df8e81e14a45 Mon Sep 17 00:00:00 2001 From: Joffrey JAFFEUX Date: Mon, 20 Jan 2025 12:00:11 +0100 Subject: [PATCH] UX: replaces custom more menu by d-menu (#29090) One of the big advantages is a nicer menu on mobile. This commit also fixes a bug where the close modal action was called for any destroyed d-menu trigger, even if this specific menu was not expanding, which means it was closing a different modal than its own modal, given we can only have one modal at a time. --- .../discourse/app/components/d-modal.gjs | 8 ++ .../header/hamburger-dropdown-wrapper.gjs | 7 +- .../components/sidebar/anonymous/sections.gjs | 5 +- .../sidebar/common/custom-section.gjs | 1 + .../sidebar/common/custom-sections.gjs | 6 +- .../components/sidebar/hamburger-dropdown.gjs | 1 + .../components/sidebar/more-section-link.gjs | 2 + .../components/sidebar/more-section-links.gjs | 129 ++++++------------ .../sidebar/section-link-button.gjs | 62 ++++++--- .../app/components/sidebar/sections.gjs | 6 +- .../app/components/sidebar/user/sections.gjs | 6 +- .../acceptance/admin-sidebar-section-test.js | 6 +- .../acceptance/enforce-second-factor-test.js | 2 +- .../tests/acceptance/meta-tag-updater-test.js | 2 +- ...idebar-anonymous-community-section-test.js | 4 +- .../sidebar-user-community-section-test.js | 68 ++++----- .../tests/acceptance/sidebar-user-test.js | 3 +- .../float-kit/addon/lib/d-menu-instance.js | 2 +- .../base/sidebar-more-section-links.scss | 40 +----- .../common/base/sidebar-section.scss | 7 +- .../common/components/dropdown-menu.scss | 1 + .../stylesheets/desktop/menu-panel.scss | 2 +- .../stylesheets/mobile/float-kit/d-menu.scss | 4 + .../components/navigation_menu/sidebar.rb | 6 +- spec/system/viewing_sidebar_spec.rb | 31 +++++ 25 files changed, 206 insertions(+), 205 deletions(-) diff --git a/app/assets/javascripts/discourse/app/components/d-modal.gjs b/app/assets/javascripts/discourse/app/components/d-modal.gjs index 0a83c77fa6f..fd6fa5164c1 100644 --- a/app/assets/javascripts/discourse/app/components/d-modal.gjs +++ b/app/assets/javascripts/discourse/app/components/d-modal.gjs @@ -158,6 +158,12 @@ export default class DModal extends Component { this.closeModal(CLOSE_INITIATED_BY_SWIPE_DOWN); } + @action + handleWrapperPointerDown(e) { + // prevents hamburger menu to close on modal backdrop click + e.stopPropagation(); + } + @action handleWrapperClick(e) { if (e.button !== 0) { @@ -396,6 +402,8 @@ export default class DModal extends Component { enabled=this.dismissable }} {{on "click" this.handleWrapperClick}} + {{! template-lint-disable no-pointer-down-event-binding }} + {{on "pointerdown" this.handleWrapperPointerDown}} > {{/unless}} diff --git a/app/assets/javascripts/discourse/app/components/header/hamburger-dropdown-wrapper.gjs b/app/assets/javascripts/discourse/app/components/header/hamburger-dropdown-wrapper.gjs index 908e46d431e..c911d80fc19 100644 --- a/app/assets/javascripts/discourse/app/components/header/hamburger-dropdown-wrapper.gjs +++ b/app/assets/javascripts/discourse/app/components/header/hamburger-dropdown-wrapper.gjs @@ -12,7 +12,7 @@ import closeOnClickOutside from "../../modifiers/close-on-click-outside"; import SidebarHamburgerDropdown from "../sidebar/hamburger-dropdown"; const CLOSE_ON_CLICK_SELECTORS = - "a[href], .sidebar-section-header-button, .sidebar-section-link-button, .sidebar-section-link"; + "a[href], .sidebar-section-header-button, .sidebar-section-link:not(.--link-button)"; export default class HamburgerDropdownWrapper extends Component { @service currentUser; @@ -37,6 +37,10 @@ export default class HamburgerDropdownWrapper extends Component { @action clickOutside(e) { + if (e.target.closest(".sidebar-more-section-content")) { + return; + } + if ( e.target.classList.contains("header-cloak") && !prefersReducedMotion() @@ -101,6 +105,7 @@ export default class HamburgerDropdownWrapper extends Component { > diff --git a/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.gjs b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.gjs index 9e3c7d0effe..730098ff0f8 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.gjs +++ b/app/assets/javascripts/discourse/app/components/sidebar/anonymous/sections.gjs @@ -9,7 +9,10 @@ export default class SidebarAnonymousSections extends Component { ; diff --git a/app/assets/javascripts/discourse/app/components/sidebar/more-section-links.gjs b/app/assets/javascripts/discourse/app/components/sidebar/more-section-links.gjs index 0ffa856d5b7..f3929a8a913 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/more-section-links.gjs +++ b/app/assets/javascripts/discourse/app/components/sidebar/more-section-links.gjs @@ -1,14 +1,14 @@ import Component from "@glimmer/component"; import { tracked } from "@glimmer/tracking"; +import { fn } from "@ember/helper"; import { on } from "@ember/modifier"; import { action } from "@ember/object"; -import didInsert from "@ember/render-modifiers/modifiers/did-insert"; -import willDestroy from "@ember/render-modifiers/modifiers/will-destroy"; import { service } from "@ember/service"; import { isEmpty } from "@ember/utils"; +import DropdownMenu from "discourse/components/dropdown-menu"; import icon from "discourse/helpers/d-icon"; -import { bind } from "discourse/lib/decorators"; import { i18n } from "discourse-i18n"; +import DMenu from "float-kit/components/d-menu"; import MoreSectionLink from "./more-section-link"; import SectionLinkButton from "./section-link-button"; @@ -16,7 +16,6 @@ export default class SidebarMoreSectionLinks extends Component { @service router; @tracked activeSectionLink; - @tracked open = false; constructor() { super(...arguments); @@ -26,7 +25,6 @@ export default class SidebarMoreSectionLinks extends Component { willDestroy() { super.willDestroy(...arguments); - this.#removeClickEventListener(); this.router.off("routeDidChange", this, this.#setActiveSectionLink); } @@ -52,53 +50,6 @@ export default class SidebarMoreSectionLinks extends Component { }); } - @bind - closeDetails(event) { - if (event.target.closest(".sidebar-more-section-links-details-summary")) { - return; - } - - if (this.open) { - const isLinkClick = - event.target.className.includes("sidebar-section-link") || - event.target.className.includes("--link-button"); - - if (isLinkClick || this.#isOutsideDetailsClick(event)) { - this.open = false; - } - } - } - - @action - registerClickListener() { - this.#addClickEventListener(); - } - - @action - unregisterClickListener() { - this.#removeClickEventListener(); - } - - @action - toggleSectionLinks(event) { - event.stopPropagation(); - this.open = !this.open; - } - - #removeClickEventListener() { - document.removeEventListener("click", this.closeDetails); - } - - #addClickEventListener() { - document.addEventListener("click", this.closeDetails); - } - - #isOutsideDetailsClick(event) { - return !event.composedPath().some((element) => { - return element.className === "sidebar-more-section-links-details"; - }); - } - #setActiveSectionLink() { this.activeSectionLink = this.args.sectionLinks.find((sectionLink) => { const args = [sectionLink.route]; @@ -117,54 +68,64 @@ export default class SidebarMoreSectionLinks extends Component { }); } + @action + closeMenu(menu) { + menu.close(); + + if (this.args.toggleNavigationMenu) { + this.args.toggleNavigationMenu(); + } + } + } diff --git a/app/assets/javascripts/discourse/app/components/sidebar/section-link-button.gjs b/app/assets/javascripts/discourse/app/components/sidebar/section-link-button.gjs index a2f11c92cf7..3502f0a91c4 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/section-link-button.gjs +++ b/app/assets/javascripts/discourse/app/components/sidebar/section-link-button.gjs @@ -1,23 +1,49 @@ +import Component from "@glimmer/component"; import { on } from "@ember/modifier"; +import { action } from "@ember/object"; +import { service } from "@ember/service"; import icon from "discourse/helpers/d-icon"; -const SidebarSectionLinkButton = ; +export default class SidebarSectionLinkButton extends Component { + @service menu; + @service header; + @service siteSettings; -export default SidebarSectionLinkButton; + @action + handleClick() { + const menuInstance = this.menu.getByIdentifier(MORE_MENU); + + this.args.action(); + + this.menu.close(menuInstance); + + if (this.args.toggleNavigationMenu) { + this.args.toggleNavigationMenu(); + } + + if (this.siteSettings.navigation_menu === "header dropdown") { + this.header.hamburgerVisible = false; + } + } + + +} diff --git a/app/assets/javascripts/discourse/app/components/sidebar/sections.gjs b/app/assets/javascripts/discourse/app/components/sidebar/sections.gjs index 1ca76ad9af3..56f46d068cd 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/sections.gjs +++ b/app/assets/javascripts/discourse/app/components/sidebar/sections.gjs @@ -7,9 +7,13 @@ const SidebarSections = ; diff --git a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.gjs b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.gjs index 714ff2fcba9..c2541e0b159 100644 --- a/app/assets/javascripts/discourse/app/components/sidebar/user/sections.gjs +++ b/app/assets/javascripts/discourse/app/components/sidebar/user/sections.gjs @@ -11,7 +11,11 @@ export default class SidebarUserSections extends Component {