shared dropdown: don't reopen when the trigger is clicked while open
mara, #3775: clicking a badge with its own dropdown open reopens it instead of closing it. Root cause: the outside-click listener only excludes the dropdown's own ref, not the sibling trigger that opened it — a click on the trigger closes via that listener (pointerdown fires first), then the trigger's own onClick toggle fires straight after and reopens it, since its closure reads the pre-close state. Dropdown now takes an optional anchorRef (the trigger's wrapper, which callers already have for CSS positioning) and excludes it from the outside-click check too — the same shape MetaNav's own hand-rolled popover already uses correctly. Wired into StatusChips's Picker + StatusMenu and swarm-ui's ComponentsPage demo (the only three Dropdown consumers). Verified two ways: reverted the fix, rebuilt, confirmed the bug reproduces via a raw-CDP interaction test (two real clicks dispatched through headless chromium, not just a static screenshot); restored the fix, rebuilt, confirmed it passes.
This commit is contained in:
parent
e8813b86eb
commit
b1254f89cd
3 changed files with 33 additions and 8 deletions
|
|
@ -10,7 +10,7 @@
|
||||||
// selection state live in the caller (`Root.tsx`, via the
|
// selection state live in the caller (`Root.tsx`, via the
|
||||||
// `useAgentState` hook), so this component can still be demoed and
|
// `useAgentState` hook), so this component can still be demoed and
|
||||||
// reviewed against plain sample data independent of live `/api/state`.
|
// reviewed against plain sample data independent of live `/api/state`.
|
||||||
import { useState } from 'preact/hooks';
|
import { useRef, useState } from 'preact/hooks';
|
||||||
import { Badge, type BadgeTone } from '@hive/shared/badge.js';
|
import { Badge, type BadgeTone } from '@hive/shared/badge.js';
|
||||||
import { Dropdown, type DropdownOption } from '@hive/shared/dropdown.js';
|
import { Dropdown, type DropdownOption } from '@hive/shared/dropdown.js';
|
||||||
import './StatusChips.css';
|
import './StatusChips.css';
|
||||||
|
|
@ -68,13 +68,14 @@ function Picker({
|
||||||
title?: string;
|
title?: string;
|
||||||
}) {
|
}) {
|
||||||
const [open, setOpen] = useState(false);
|
const [open, setOpen] = useState(false);
|
||||||
|
const anchorRef = useRef<HTMLDivElement>(null);
|
||||||
const dropdownOptions: DropdownOption[] = options.map((name) => ({
|
const dropdownOptions: DropdownOption[] = options.map((name) => ({
|
||||||
value: name,
|
value: name,
|
||||||
label: name,
|
label: name,
|
||||||
description: descriptions[name] || undefined,
|
description: descriptions[name] || undefined,
|
||||||
}));
|
}));
|
||||||
return (
|
return (
|
||||||
<div class="status-chip-anchor">
|
<div class="status-chip-anchor" ref={anchorRef}>
|
||||||
<Badge label={label} value={value} onClick={() => setOpen((o) => !o)} expanded={open} title={title} />
|
<Badge label={label} value={value} onClick={() => setOpen((o) => !o)} expanded={open} title={title} />
|
||||||
<Dropdown
|
<Dropdown
|
||||||
open={open}
|
open={open}
|
||||||
|
|
@ -86,6 +87,7 @@ function Picker({
|
||||||
setOpen(false);
|
setOpen(false);
|
||||||
}}
|
}}
|
||||||
onClose={() => setOpen(false)}
|
onClose={() => setOpen(false)}
|
||||||
|
anchorRef={anchorRef}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
|
|
@ -112,6 +114,7 @@ function StatusMenu({
|
||||||
>) {
|
>) {
|
||||||
const [open, setOpen] = useState(false);
|
const [open, setOpen] = useState(false);
|
||||||
const [confirmCancel, setConfirmCancel] = useState(false);
|
const [confirmCancel, setConfirmCancel] = useState(false);
|
||||||
|
const anchorRef = useRef<HTMLDivElement>(null);
|
||||||
|
|
||||||
function close() {
|
function close() {
|
||||||
setOpen(false);
|
setOpen(false);
|
||||||
|
|
@ -128,7 +131,7 @@ function StatusMenu({
|
||||||
}
|
}
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<div class="status-chip-anchor">
|
<div class="status-chip-anchor" ref={anchorRef}>
|
||||||
<Badge
|
<Badge
|
||||||
value={statusLabel}
|
value={statusLabel}
|
||||||
tone={statusTone}
|
tone={statusTone}
|
||||||
|
|
@ -155,6 +158,7 @@ function StatusMenu({
|
||||||
close();
|
close();
|
||||||
}}
|
}}
|
||||||
onClose={close}
|
onClose={close}
|
||||||
|
anchorRef={anchorRef}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
|
|
@ -15,7 +15,7 @@
|
||||||
// document-flow header never needs one, and skipping it keeps focus
|
// document-flow header never needs one, and skipping it keeps focus
|
||||||
// management simple (no re-parenting to `<body>` to reason about).
|
// management simple (no re-parenting to `<body>` to reason about).
|
||||||
import { useEffect, useRef } from 'preact/hooks';
|
import { useEffect, useRef } from 'preact/hooks';
|
||||||
import type { ComponentChildren } from 'preact';
|
import type { ComponentChildren, RefObject } from 'preact';
|
||||||
import './Dropdown.css';
|
import './Dropdown.css';
|
||||||
|
|
||||||
export interface DropdownOption {
|
export interface DropdownOption {
|
||||||
|
|
@ -35,15 +35,34 @@ export interface DropdownProps {
|
||||||
onClose: () => void;
|
onClose: () => void;
|
||||||
/** `aria-label` for the option list (e.g. "select model"). */
|
/** `aria-label` for the option list (e.g. "select model"). */
|
||||||
label: string;
|
label: string;
|
||||||
|
/**
|
||||||
|
* Ref to the trigger badge/button that opened this dropdown. Without
|
||||||
|
* it, a click on the trigger while open closes-then-reopens instead
|
||||||
|
* of closing: the trigger's own `onClick` toggles `open`, but this
|
||||||
|
* component's outside-click listener doesn't know the trigger is
|
||||||
|
* "part of" the dropdown (it's a sibling, not a descendant of this
|
||||||
|
* `<div>`), so it *also* fires `onClose` on the same click — the two
|
||||||
|
* updates race, and the toggle's `!open` reads the pre-close value
|
||||||
|
* and wins, reopening it (mara: "clicking badge with open drop down
|
||||||
|
* reopens it instead of closing it"). Passing the same ref the
|
||||||
|
* caller's wrapper div already needs for CSS positioning excludes
|
||||||
|
* clicks on the trigger from the outside-click check, matching how
|
||||||
|
* `MetaNav`'s own hand-rolled popover (which wraps trigger+popover in
|
||||||
|
* one ref) already avoids this.
|
||||||
|
*/
|
||||||
|
anchorRef?: RefObject<HTMLElement>;
|
||||||
}
|
}
|
||||||
|
|
||||||
export function Dropdown({ open, options, activeValue, onSelect, onClose, label }: DropdownProps) {
|
export function Dropdown({ open, options, activeValue, onSelect, onClose, label, anchorRef }: DropdownProps) {
|
||||||
const ref = useRef<HTMLDivElement>(null);
|
const ref = useRef<HTMLDivElement>(null);
|
||||||
|
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (!open) return;
|
if (!open) return;
|
||||||
function handlePointerDown(e: PointerEvent) {
|
function handlePointerDown(e: PointerEvent) {
|
||||||
if (ref.current && e.target instanceof Node && !ref.current.contains(e.target)) onClose();
|
if (!(e.target instanceof Node)) return;
|
||||||
|
if (ref.current?.contains(e.target)) return;
|
||||||
|
if (anchorRef?.current?.contains(e.target)) return;
|
||||||
|
onClose();
|
||||||
}
|
}
|
||||||
function handleKeyDown(e: KeyboardEvent) {
|
function handleKeyDown(e: KeyboardEvent) {
|
||||||
if (e.key === 'Escape') onClose();
|
if (e.key === 'Escape') onClose();
|
||||||
|
|
|
||||||
|
|
@ -5,7 +5,7 @@
|
||||||
// primitive gets a section here the same day it's added. Sample data
|
// primitive gets a section here the same day it's added. Sample data
|
||||||
// only, no network calls — this page must render the same whether
|
// only, no network calls — this page must render the same whether
|
||||||
// swarm-controller's API is up or not.
|
// swarm-controller's API is up or not.
|
||||||
import { useState } from 'preact/hooks';
|
import { useRef, useState } from 'preact/hooks';
|
||||||
import type { ComponentChildren } from 'preact';
|
import type { ComponentChildren } from 'preact';
|
||||||
import { Panel } from '../ui/panel/Panel.js';
|
import { Panel } from '../ui/panel/Panel.js';
|
||||||
import { RelativeTime } from '../ui/relative-time/RelativeTime.js';
|
import { RelativeTime } from '../ui/relative-time/RelativeTime.js';
|
||||||
|
|
@ -101,8 +101,9 @@ function RefreshIntervalPickerSample() {
|
||||||
function BadgePickerSample() {
|
function BadgePickerSample() {
|
||||||
const [value, setValue] = useState('sonnet');
|
const [value, setValue] = useState('sonnet');
|
||||||
const [open, setOpen] = useState(false);
|
const [open, setOpen] = useState(false);
|
||||||
|
const anchorRef = useRef<HTMLDivElement>(null);
|
||||||
return (
|
return (
|
||||||
<div class="components-badge-anchor">
|
<div class="components-badge-anchor" ref={anchorRef}>
|
||||||
<Badge
|
<Badge
|
||||||
label="model"
|
label="model"
|
||||||
value={value}
|
value={value}
|
||||||
|
|
@ -119,6 +120,7 @@ function BadgePickerSample() {
|
||||||
setOpen(false);
|
setOpen(false);
|
||||||
}}
|
}}
|
||||||
onClose={() => setOpen(false)}
|
onClose={() => setOpen(false)}
|
||||||
|
anchorRef={anchorRef}
|
||||||
/>
|
/>
|
||||||
</div>
|
</div>
|
||||||
);
|
);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue