From a29111473e7d4924aab2b00fbb4d4e9ffd00255e Mon Sep 17 00:00:00 2001 From: SylvainDruaux Date: Wed, 28 Oct 2026 09:50:20 +0100 Subject: [PATCH] fix(welcome): act on the split button's main half while its menu is open With a split button's menu open, a click on its main half only closed the menu, because the popup treated it as an outside press and swallowed it. Popup now takes an optional within element whose presses are not outside presses, and the split button passes its whole group, so one click closes the menu and runs the main action. --- src/features/welcome/WelcomeSplitButton.tsx | 9 ++++++++- src/ui/Popup.tsx | 3 +++ src/ui/popupDismiss.ts | 12 ++++++++---- test/renderer/welcome/welcome.test.js | 14 ++++++++++++++ 4 files changed, 33 insertions(+), 5 deletions(-) diff --git a/src/features/welcome/WelcomeSplitButton.tsx b/src/features/welcome/WelcomeSplitButton.tsx index 0bea5453..da68836b 100644 --- a/src/features/welcome/WelcomeSplitButton.tsx +++ b/src/features/welcome/WelcomeSplitButton.tsx @@ -19,7 +19,13 @@ export default function WelcomeSplitButton(props: WelcomeSplitButtonProps) { const menuId = useId(); return (
- @@ -45,6 +51,7 @@ export default function WelcomeSplitButton(props: WelcomeSplitButtonProps) { portal={false} open={open} anchor={triggerRef.current ?? undefined} + within={groupRef.current ?? undefined} onClose={() => setOpen(false)} className="welcome-split-menu" role="menu" diff --git a/src/ui/Popup.tsx b/src/ui/Popup.tsx index 4b5a3150..f030d279 100644 --- a/src/ui/Popup.tsx +++ b/src/ui/Popup.tsx @@ -36,6 +36,9 @@ export interface PopupProps extends Pick< readonly onEscape?: () => void; readonly dismissible?: boolean; readonly anchor?: HTMLElement | undefined; + // A press inside this element is not an outside press: the popup stays open and the click + // goes through, so a control beside the anchor (a split button's main half) acts at once. + readonly within?: HTMLElement | undefined; readonly className?: string; readonly style?: CSSProperties | undefined; readonly role?: 'dialog' | 'alertdialog' | 'menu' | 'listbox' | 'tooltip'; diff --git a/src/ui/popupDismiss.ts b/src/ui/popupDismiss.ts index 26d63ec8..3bb433c2 100644 --- a/src/ui/popupDismiss.ts +++ b/src/ui/popupDismiss.ts @@ -2,7 +2,10 @@ import { inOwnedPopup, isTopPopupLayer } from './popupOwnership'; import { useEffect, type RefObject } from 'react'; import type { PopupProps, PopupActions } from './Popup'; import { toRecord } from '../../shared/core/record'; -interface DismissOptions extends Pick { +interface DismissOptions extends Pick< + PopupProps, + 'dismissible' | 'anchor' | 'within' | 'onEscape' +> { readonly active: boolean; readonly mode: 'modal' | 'popover'; } @@ -11,7 +14,7 @@ export function usePopupDismiss( options: DismissOptions, { close, finish }: PopupActions, ): void { - const { active, anchor, dismissible = true, mode, onEscape } = options; + const { active, anchor, within, dismissible = true, mode, onEscape } = options; usePopupEnvironment(surface, options, close); useEffect(() => { if (!active || !dismissible) { @@ -23,7 +26,8 @@ export function usePopupDismiss( !(target instanceof window.Element && target.closest('.toast-stack')) && !surface.current?.contains(target) && !inOwnedPopup(target, surface.current ?? undefined) && - !anchor?.contains(target); + !anchor?.contains(target) && + !within?.contains(target); const down = (event: MouseEvent): void => { outsidePress = outside(event.target); if (mode === 'popover' && outsidePress && popupIsTop(surface.current)) { @@ -75,7 +79,7 @@ export function usePopupDismiss( document.removeEventListener('keydown', key); window.removeEventListener('message', message); }; - }, [active, anchor, close, finish, dismissible, mode, onEscape, surface]); + }, [active, anchor, within, close, finish, dismissible, mode, onEscape, surface]); } export function popupIsTop(surface: RefObject['current']): boolean { diff --git a/test/renderer/welcome/welcome.test.js b/test/renderer/welcome/welcome.test.js index a94dbe3a..a209c96f 100644 --- a/test/renderer/welcome/welcome.test.js +++ b/test/renderer/welcome/welcome.test.js @@ -209,6 +209,20 @@ const PIXEL = document.body.dispatchEvent(new dom.window.MouseEvent('mousedown', { bubbles: true })); }); check('an outside click closes the menu', all('[role=menuitem]').length === 0); + // The main half beside an open menu is part of the same control: its press is not an + // outside press, so the first click opens the picker instead of only closing the menu. + await act(async () => arrows[0].click()); + const openFromFolder = all('.actions button')[0]; + await act(async () => { + openFromFolder.dispatchEvent(new dom.window.MouseEvent('mousedown', { bubbles: true })); + openFromFolder.click(); + }); + check( + 'with its menu open, Open from folder opens the picker on the first click', + folderRequests === 2, + String(folderRequests), + ); + check('and closes the menu', all('[role=menuitem]').length === 0); check('nothing tells the user a picture is old', !/out of date/i.test(text()), text()); check('and nothing offers to fix it', all('.recent-refresh').length === 0);