From f4196a88432d983c337b24845301dcfd36f36feb Mon Sep 17 00:00:00 2001 From: Kerwin Bryant Date: Sun, 13 Apr 2025 09:59:36 +0800 Subject: [PATCH] Optimize overflow-menu (#34183) Optimized the overflow-menu: 1. Close the tippy when a menu item inside the tippy is clicked. 2. When a menu item inside the tippy is selected, move the active state of the menu to the tippy's button. --------- Co-authored-by: wxiaoguang --- web_src/css/base.css | 8 ++++ web_src/js/utils/dom.ts | 2 +- web_src/js/webcomponents/overflow-menu.ts | 45 ++++++++++++++++------- 3 files changed, 40 insertions(+), 15 deletions(-) diff --git a/web_src/css/base.css b/web_src/css/base.css index 37ee7f5832..353ae851ad 100644 --- a/web_src/css/base.css +++ b/web_src/css/base.css @@ -747,6 +747,14 @@ overflow-menu .overflow-menu-button { padding: 0; } +/* match the styles of ".ui.secondary.pointing.menu .active.item" */ +overflow-menu.ui.secondary.pointing.menu .overflow-menu-button.active { + padding: 2px 0 0; + border-bottom: 2px solid currentcolor; + background-color: transparent; + font-weight: var(--font-weight-medium); +} + overflow-menu .overflow-menu-button:hover { color: var(--color-text-dark); } diff --git a/web_src/js/utils/dom.ts b/web_src/js/utils/dom.ts index b3debfde9e..98e5170a2b 100644 --- a/web_src/js/utils/dom.ts +++ b/web_src/js/utils/dom.ts @@ -360,7 +360,7 @@ export function querySingleVisibleElem(parent: Element, s export function addDelegatedEventListener(parent: Node, type: string, selector: string, listener: (elem: T, e: E) => Promisable, options?: boolean | AddEventListenerOptions) { parent.addEventListener(type, (e: Event) => { const elem = (e.target as HTMLElement).closest(selector); - if (!elem) return; + if (!elem || !parent.contains(elem)) return; listener(elem as T, e as E); }, options); } diff --git a/web_src/js/webcomponents/overflow-menu.ts b/web_src/js/webcomponents/overflow-menu.ts index 4e729a268a..ae93f2b758 100644 --- a/web_src/js/webcomponents/overflow-menu.ts +++ b/web_src/js/webcomponents/overflow-menu.ts @@ -1,6 +1,6 @@ import {throttle} from 'throttle-debounce'; import {createTippy} from '../modules/tippy.ts'; -import {isDocumentFragmentOrElementNode} from '../utils/dom.ts'; +import {addDelegatedEventListener, isDocumentFragmentOrElementNode} from '../utils/dom.ts'; import octiconKebabHorizontal from '../../../public/assets/img/svg/octicon-kebab-horizontal.svg'; window.customElements.define('overflow-menu', class extends HTMLElement { @@ -12,10 +12,14 @@ window.customElements.define('overflow-menu', class extends HTMLElement { mutationObserver: MutationObserver; lastWidth: number; + updateButtonActivationState() { + if (!this.button || !this.tippyContent) return; + this.button.classList.toggle('active', Boolean(this.tippyContent.querySelector('.item.active'))); + } + updateItems = throttle(100, () => { if (!this.tippyContent) { const div = document.createElement('div'); - div.classList.add('tippy-target'); div.tabIndex = -1; // for initial focus, programmatic focus only div.addEventListener('keydown', (e) => { if (e.key === 'Tab') { @@ -64,9 +68,10 @@ window.customElements.define('overflow-menu', class extends HTMLElement { } } }); - this.append(div); + div.classList.add('tippy-target'); + this.handleItemClick(div, '.tippy-target > .item'); this.tippyContent = div; - } + } // end if: no tippyContent and create a new one const itemFlexSpace = this.menuItemsEl.querySelector('.item-flex-space'); const itemOverFlowMenuButton = this.querySelector('.overflow-menu-button'); @@ -88,7 +93,7 @@ window.customElements.define('overflow-menu', class extends HTMLElement { const menuRight = this.offsetLeft + this.offsetWidth; const menuItems = this.menuItemsEl.querySelectorAll('.item, .item-flex-space'); let afterFlexSpace = false; - for (const item of menuItems) { + for (const [idx, item] of menuItems.entries()) { if (item.classList.contains('item-flex-space')) { afterFlexSpace = true; continue; @@ -96,7 +101,10 @@ window.customElements.define('overflow-menu', class extends HTMLElement { if (afterFlexSpace) item.setAttribute('data-after-flex-space', 'true'); const itemRight = item.offsetLeft + item.offsetWidth; if (menuRight - itemRight < 38) { // roughly the width of .overflow-menu-button with some extra space - this.tippyItems.push(item); + const onlyLastItem = idx === menuItems.length - 1 && this.tippyItems.length === 0; + const lastItemFit = onlyLastItem && menuRight - itemRight > 0; + const moveToPopup = !onlyLastItem || !lastItemFit; + if (moveToPopup) this.tippyItems.push(item); } } itemFlexSpace?.style.removeProperty('display'); @@ -107,6 +115,7 @@ window.customElements.define('overflow-menu', class extends HTMLElement { const btn = this.querySelector('.overflow-menu-button'); btn?._tippy?.destroy(); btn?.remove(); + this.button = null; return; } @@ -126,18 +135,17 @@ window.customElements.define('overflow-menu', class extends HTMLElement { // update existing tippy if (this.button?._tippy) { this.button._tippy.setContent(this.tippyContent); + this.updateButtonActivationState(); return; } // create button initially - const btn = document.createElement('button'); - btn.classList.add('overflow-menu-button'); - btn.setAttribute('aria-label', window.config.i18n.more_items); - btn.innerHTML = octiconKebabHorizontal; - this.append(btn); - this.button = btn; - - createTippy(btn, { + this.button = document.createElement('button'); + this.button.classList.add('overflow-menu-button'); + this.button.setAttribute('aria-label', window.config.i18n.more_items); + this.button.innerHTML = octiconKebabHorizontal; + this.append(this.button); + createTippy(this.button, { trigger: 'click', hideOnClick: true, interactive: true, @@ -151,6 +159,7 @@ window.customElements.define('overflow-menu', class extends HTMLElement { }, 0); }, }); + this.updateButtonActivationState(); }); init() { @@ -187,6 +196,14 @@ window.customElements.define('overflow-menu', class extends HTMLElement { } }); this.resizeObserver.observe(this); + this.handleItemClick(this, '.overflow-menu-items > .item'); + } + + handleItemClick(el: Element, selector: string) { + addDelegatedEventListener(el, 'click', selector, () => { + this.button?._tippy?.hide(); + this.updateButtonActivationState(); + }); } connectedCallback() {