settings menu: replace showExpandDetails prop with a children slot
mara, reviewing PR#3780's showExpandDetails boolean prop: "you cannot just add it like this - if we have more and more options there in different places we will keep accumulating cruft in the shared component." SettingsMenu now exposes a plain children slot instead; the agent page owns its expand-tool-output row entirely (state, storage, markup) in its own ExpandDetailsSetting component and passes it in, so the shared component never learns that setting exists.
This commit is contained in:
parent
385d4b6fd7
commit
33fafc8c19
3 changed files with 52 additions and 31 deletions
|
|
@ -11,6 +11,7 @@ import { useApplyThemeOverride } from '@hive/shared/theme-apply.js';
|
|||
import { useApplyMotionOverride } from '@hive/shared/motion-apply.js';
|
||||
import { Header } from './components/Header.js';
|
||||
import { MetaNav } from './components/MetaNav.js';
|
||||
import { ExpandDetailsSetting } from './components/ExpandDetailsSetting.js';
|
||||
import { StatusChips } from './components/StatusChips.js';
|
||||
import { LiveStream, type LiveStreamHandle } from './components/LiveStream.js';
|
||||
import { HeaderPill } from './components/HeaderPill.js';
|
||||
|
|
@ -113,7 +114,9 @@ export function Root() {
|
|||
dashboardBase={resolveDashboardBase(state.dashboard_port)}
|
||||
/>
|
||||
) : null}
|
||||
<SettingsMenu themeKey={THEME_KEY} motionKey={MOTION_KEY} showExpandDetails />
|
||||
<SettingsMenu themeKey={THEME_KEY} motionKey={MOTION_KEY}>
|
||||
<ExpandDetailsSetting />
|
||||
</SettingsMenu>
|
||||
</>
|
||||
);
|
||||
// Kept mounted regardless of `openPanel` (open/closed toggles just the
|
||||
|
|
|
|||
|
|
@ -0,0 +1,35 @@
|
|||
// Agent-terminal-only settings-menu row: "expand tool output panels".
|
||||
// Rendered as a `<SettingsMenu>` child (`@hive/shared/settings-menu.js`)
|
||||
// rather than a prop on that shared component — `SettingsMenu` is also
|
||||
// used by swarm-ui, which has no terminal to apply this to, and a prior
|
||||
// attempt to gate it behind a boolean prop (`showExpandDetails`) on the
|
||||
// shared component drew mara's review: "you cannot just add it like
|
||||
// this - if we have more and more options there in different places we
|
||||
// will keep accumulating cruft in the shared component." This file owns
|
||||
// its own state/storage entirely; `SettingsMenu` just renders it as a
|
||||
// child, no different from any other consumer's markup.
|
||||
import { useState } from 'preact/hooks';
|
||||
import { getExpandDetailsPref, setExpandDetailsPref } from '@hive/shared/prefs.js';
|
||||
|
||||
export function ExpandDetailsSetting() {
|
||||
// Plain localStorage read, not a hook-managed override like
|
||||
// theme/motion — `getExpandDetailsPref`/`setExpandDetailsPref`
|
||||
// (`@hive/shared/prefs.js`) are the existing shared-storage-key pair
|
||||
// the terminal itself already reads live (no round-trip needed here
|
||||
// beyond re-rendering this row's own checkbox state on toggle).
|
||||
const [expandDetails, setExpandDetailsState] = useState(() => getExpandDetailsPref());
|
||||
return (
|
||||
<label class="settings-menu-row">
|
||||
<span>expand tool output</span>
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={expandDetails}
|
||||
onChange={(e) => {
|
||||
const v = (e.target as HTMLInputElement).checked;
|
||||
setExpandDetailsPref(v);
|
||||
setExpandDetailsState(v);
|
||||
}}
|
||||
/>
|
||||
</label>
|
||||
);
|
||||
}
|
||||
|
|
@ -24,11 +24,11 @@
|
|||
// component, so the two stay in sync without this component owning any
|
||||
// page-specific naming decision.
|
||||
import { useEffect, useRef, useState } from 'preact/hooks';
|
||||
import type { ComponentChildren } from 'preact';
|
||||
import { Badge } from '../badge/Badge.js';
|
||||
import { GearIcon } from '../icons.js';
|
||||
import { useThemeOverride, type ThemeOverride } from './theme-apply.js';
|
||||
import { useMotionOverride, type MotionOverride } from './motion-apply.js';
|
||||
import { getExpandDetailsPref, setExpandDetailsPref } from '../prefs.js';
|
||||
import './SettingsMenu.css';
|
||||
|
||||
const THEME_OPTIONS: ThemeOverride[] = ['system', 'light', 'dark'];
|
||||
|
|
@ -38,28 +38,24 @@ export interface SettingsMenuProps {
|
|||
themeKey: string;
|
||||
motionKey: string;
|
||||
/**
|
||||
* Show the "expand tool output panels" toggle — agent-terminal-only,
|
||||
* formerly its own section on the (separate) hive dashboard's
|
||||
* `/settings.html`. `SettingsMenu` is shared with swarm-ui too, which
|
||||
* has no terminal to apply this to (mara: "settigs dropdown is
|
||||
* shared component, but the setting is only in the agent term") — so
|
||||
* it's opt-in per consumer rather than always rendered. Only the
|
||||
* agent page passes this; swarm-ui's `Shell` doesn't.
|
||||
* Consumer-owned extra row(s), rendered inside the popover after the
|
||||
* built-in theme/motion rows. Not a named boolean prop per setting —
|
||||
* a `showExpandDetails?: boolean` attempt drew mara's review: "you
|
||||
* cannot just add it like this - if we have more and more options
|
||||
* there in different places we will keep accumulating cruft in the
|
||||
* shared component." A caller owns its row(s) entirely (state,
|
||||
* storage key, markup); this component never learns they exist. Use
|
||||
* `settings-menu-row` (this file's CSS) on each row's outer element
|
||||
* to match spacing/typography — see `Root.tsx` for a worked example.
|
||||
*/
|
||||
showExpandDetails?: boolean;
|
||||
children?: ComponentChildren;
|
||||
}
|
||||
|
||||
export function SettingsMenu({ themeKey, motionKey, showExpandDetails }: SettingsMenuProps) {
|
||||
export function SettingsMenu({ themeKey, motionKey, children }: SettingsMenuProps) {
|
||||
const [open, setOpen] = useState(false);
|
||||
const rootRef = useRef<HTMLDivElement>(null);
|
||||
const [theme, setTheme] = useThemeOverride(themeKey);
|
||||
const [motion, setMotion] = useMotionOverride(motionKey);
|
||||
// Plain localStorage read, not a hook-managed override like theme/motion
|
||||
// above — `getExpandDetailsPref`/`setExpandDetailsPref` (`../prefs.js`)
|
||||
// are the existing shared-storage-key pair the terminal itself already
|
||||
// reads live (no round-trip needed here beyond re-rendering this menu's
|
||||
// own checkbox state on toggle).
|
||||
const [expandDetails, setExpandDetailsState] = useState(() => getExpandDetailsPref());
|
||||
|
||||
// Close on an outside click or Escape — only listens while open.
|
||||
useEffect(() => {
|
||||
|
|
@ -115,20 +111,7 @@ export function SettingsMenu({ themeKey, motionKey, showExpandDetails }: Setting
|
|||
))}
|
||||
</select>
|
||||
</label>
|
||||
{showExpandDetails ? (
|
||||
<label class="settings-menu-row">
|
||||
<span>expand tool output</span>
|
||||
<input
|
||||
type="checkbox"
|
||||
checked={expandDetails}
|
||||
onChange={(e) => {
|
||||
const v = (e.target as HTMLInputElement).checked;
|
||||
setExpandDetailsPref(v);
|
||||
setExpandDetailsState(v);
|
||||
}}
|
||||
/>
|
||||
</label>
|
||||
) : null}
|
||||
{children}
|
||||
</div>
|
||||
) : null}
|
||||
</div>
|
||||
|
|
|
|||
Loading…
Reference in a new issue