rt/wrolpi
1
0
Fork 0
mirror of https://github.com/lrnselfreliance/wrolpi.git synced 2026-10-26 19:01:39 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 1

ui: the corner triggers, the More button, and the search modal

Browse source 
Four faults, each measured in the running app before and after.

The search glyph carried Semantic's `item` class, which has had no rules at all
since Semantic was removed: no pointer cursor, no hover, and an 18.6px hit area
against a 54.4px bar.  It is the same kind of thing as Help and Admin and now
takes the same class.  The share glyph did compute `cursor: pointer` -- it is an
`<a href>` -- but its target was the 24px box the icon occupies, so the pointer
appeared only if you found it exactly.  Corner anchors now stretch and pad like
a link.

The More button rendered with NO class at all: a bare grey UA button while the
rest of the bar took the user's colour.  Menu.Target clones its child and hands
it a className of its own, and the trigger spread those cloned props after its
own `className` attribute, so the clone replaced ours.  It is merged now.  This
was mine, from extracting NavDropdownTrigger last commit -- and it also made the
previous commit's "measured and drawn are the same component" claim only half
true, because the hidden placeholder is not cloned and so kept its class.

The bar still wrapped, for a reason the padding fix could not reach: the corner
does not keep a constant width.  The Cmd-K hint renders only once
useKeyboardDetected has seen a keypress, so it appears at an arbitrary later
moment and widens the corner by ~30px; the load, memory, temperature and drive
warnings come and go with the machine's state.  None of that resizes the
container, which was the only thing observed -- so nothing recalculated.  That
is the other half of "at certain widths": the same viewport wrapped or not
depending on whether the user had touched a key.  The corner and the home link
are observed too now.

The search modal showed an input and the first line of the first group.  The
suggestion list is absolutely positioned, which is right everywhere else -- a
list that reflows the page while you type is unusable -- but in a modal it fails
twice: an absolute list adds no height, so the panel sized itself to the search
box alone at 114px, and the panel's own `overflow: auto` then clipped the list
at that edge.  In the modal the list sits in flow; the inverse is asserted so
the rule cannot leak to the pages where overlaying is correct.

Two of the new tests were wrong first and reverting the fix left them green:
they asserted hand-written markup carrying the classes I expected the components
to have, rather than the real SearchIconButton and ShareButton.  They use the
real components now.  Every fix was then planted against: 5 red for the More
class, 2 for the search class, 2 for the corner hit area, 1 for the observer,
2 for the modal.

1115 jest / 62 suites, 196 in ui-layout.cy.js, clean npm run build.
Confirmed live: share and search both pointer at 54.4px, More white on the bar
colour with a transparent face, and the modal's list in flow and unclipped.
This commit is contained in:
Roland 2026-08-02 14:45:49 -06:00
parent cf5f4d75fc
commit 8aa9725feb
No known key found for this signature in database
GPG key ID: 1BA620CE6C9A4680
8 changed files with 431 additions and 7 deletions
Download patch file Download diff file Expand all files Collapse all files

24
app/src/App.css
View file

@ -79,6 +79,30 @@ html .wrolpi-navbar .mantine-ActionIcon-root:hover {
display: flex;
align-items: center;
margin-left: 1.5em;
/* Full bar height, so the anchor inside has somewhere to stretch to. */
align-self: stretch;
}
/*
* An icon-only trigger in the corner gets a link's hit area, not its glyph's.
*
* The share glyph already computed `cursor: pointer` -- it is an `<a href>` -- but the
* target was the 24px box the icon occupies, so the pointer appeared only if you found it
* exactly, and there was no hover highlight to aim at. Stretched and padded, it behaves
* like Help and Admin beside it.
*
* `align-self` rather than `align-items: stretch` on the wrapper: the IconButtons in the
* corner have an explicit height, and a stretch container places an item it cannot stretch
* at the start -- which would push the hotspot and theme glyphs back off centre.
*/
.wrolpi-navbar-icon > a {
align-self: stretch;
padding: 0 0.5em;
cursor: pointer;
}
.wrolpi-navbar-icon > a:hover {
background: rgba(0, 0, 0, 0.12);
}
/*

11
app/src/components/KeyboardShortcutsProvider.js
View file

@ -83,6 +83,16 @@ function SearchModal({open, onClose}) {
return (
<Modal open={open} onClose={onClose} centered={false} title='Search'>
<Modal.Content>
{/*
* `wrolpi-search-modal` makes the suggestion list part of the panel rather
* than an overlay on the page; see ui.css. Everywhere else the list floats
* over the page, which is right -- results that reflow the page while you
* type are unusable. Inside a modal it is wrong twice over: an absolutely
* positioned list adds no height, so the panel stayed 114px tall, and the
* panel's own `overflow: auto` then clipped the list at the panel's edge.
* The modal showed a search box and the first word of the first result.
*/}
<div className='wrolpi-search-modal'>
<SearchResultsInput
clearable
searchStr={searchStr}
@ -97,6 +107,7 @@ function SearchModal({open, onClose}) {
inputRef={inputRef}
autoFocus
/>
</div>
</Modal.Content>
</Modal>
);

14
app/src/components/Nav.js
View file

@ -111,8 +111,18 @@ function DropdownMenuItem({link}) {
* button that eventually rendered -- the row overflowed by that much and the bar wrapped.
* Measuring one thing and drawing another is only ever right by coincidence.
*/
const NavDropdownTrigger = React.forwardRef(({text, ...props}, ref) =>
<button type='button' ref={ref} className='wrolpi-navbar-link wrolpi-navbar-link-button'
const NavDropdownTrigger = React.forwardRef(({text, className, ...props}, ref) =>
/*
* `className` is pulled out and MERGED, never spread over. Menu.Target clones its child
* and hands it a className of its own, and a `{...props}` spread placed after the
* attribute replaces ours with it -- which left the real More button with no class at
* all, so it rendered as a bare grey UA button instead of taking the bar's colour, its
* padding and its hover. The hidden placeholder is not cloned by Menu.Target, so it
* kept its class and measured wider than the button that actually drew.
*/
<button type='button' ref={ref}
className={['wrolpi-navbar-link', 'wrolpi-navbar-link-button', className]
.filter(Boolean).join(' ')}
{...props}>
{text}
<Icon name='dropdown' size='small' style={{marginLeft: '0.35em'}}/>

11
app/src/components/Search.js
View file

@ -513,11 +513,18 @@ export function SearchView({suggestions, suggestionsSums, loading}) {
}
export function SearchIconButton() {
// A single button which opens the search modal via keyboard shortcuts context.
/*
* A single button which opens the search modal via keyboard shortcuts context.
*
* `wrolpi-navbar-link`, not Semantic's leftover `item`, which carried no rules at all
* once Semantic was removed: no pointer cursor, no hover highlight, and a hit area of
* whatever the 18px glyph occupied rather than the height of the bar. It is the same
* kind of thing as Help and Admin and now has the same affordance.
*/
const {openSearchModal} = React.useContext(KeyboardShortcutsContext);
return (
<a className='item' style={{paddingRight: '0.7em'}} onClick={openSearchModal}
<a className='wrolpi-navbar-link' onClick={openSearchModal}
role='button' tabIndex={0} aria-label='Search'
onKeyDown={e => (e.key === 'Enter' || e.key === ' ') && openSearchModal()}>
<Icon name='search'/>

2
app/src/components/ThemeSamplePage.js
View file

@ -120,7 +120,7 @@ export const NavBarSample = ({color}) => {
<Icon name='warning sign' size='large' label='Drive health warning'/>
<Icon name='hdd' size='large' label='Drive temperature warning'/>
{/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
<a className='item' role='button' tabIndex={0} aria-label='Search'>
<a className='wrolpi-navbar-link' role='button' tabIndex={0} aria-label='Search'>
<Icon name='search' size='large'/>
</a>
<IconButton icon={IconMenu2} label='Menu' variant='subtle'/>

329
app/src/components/ui/ui-layout.cy.js
View file

@ -13,6 +13,7 @@ import {monochromeThemes, themeNames} from '../../themes/names';
import {navColorNames} from '../../themes/navColors';
import {NavBarSample} from '../ThemeSamplePage';
import {DesktopNav, NavIconWrapper} from '../Nav';
import {ShareButton} from '../Share';
/* `--panel` is authored as a hex; computed backgrounds come back as rgb(). */
const hexToRgb = (hex) => {
@ -2403,3 +2404,331 @@ describe('the navigation bar never breaks onto a second line', () => {
});
});
});
describe('the navbar overflow menu keeps the bar\'s styling', () => {
/*
* Mantine's Menu.Target clones its child and hands it a className of its own. The
* trigger spread those cloned props AFTER its own `className` attribute, so the cloned
* value replaced ours and the real More button rendered with NO class at all -- a bare
* UA button, grey on dark and white on light, while every other item in the bar took the
* user's navbar colour.
*
* The hidden placeholder DesktopNav measures is NOT cloned by Menu.Target, so it kept its
* class and stayed the width the styled button would have been. That is why the width
* sweep passed while the visible button was wrong: the two were the same component and
* still not the same thing.
*/
const themeIcon = <NavIconWrapper>
<IconButton icon='sun' label='Theme' variant='subtle'/>
</NavIconWrapper>;
// Narrow enough that links have to move into More.
const mountNarrow = (theme) => cy.mountUI(
<MemoryRouter><div style={{width: 700}}>
<DesktopNav
navColors={{background: '#5c4fa8', color: '#ffffff', ratio: 6.69}}
homeLink={<a className='wrolpi-navbar-link' href='#'><i>WROLPi</i></a>}
icons={themeIcon}
/>
</div></MemoryRouter>,
{theme},
);
const moreButton = () => cy.contains('.wrolpi-navbar button', 'More');
themeNames.forEach((theme) => {
it(`draws More in the bar's own colour in ${theme}`, () => {
mountNarrow(theme);
cy.get('.wrolpi-navbar').then(($nav) => {
const expected = getComputedStyle($nav[0]).color;
moreButton().should(($more) => {
const style = getComputedStyle($more[0]);
expect($more[0].className, 'More carries the bar classes')
.to.contain('wrolpi-navbar-link');
expect(style.color, `More text colour in ${theme}`).to.equal(expected);
// A UA button paints a grey face; the bar's items are transparent.
expect(style.backgroundColor, `More background in ${theme}`)
.to.equal('rgba(0, 0, 0, 0)');
});
});
});
});
it('gives More the same hit area and caret as the placeholder it was measured as', () => {
// The measurement contract. If the drawn button is narrower or shorter than the
// space reserved for it, the row overflows and the bar wraps.
mountNarrow('light');
/*
* Both measurements inside the retried assertion. Reading the bar's height in a
* plain `.then()` first captures whatever it was at that instant -- including the
* transient wrapped state while the corner is still settling -- and then retries the
* comparison against that stale number forever.
*/
cy.get('.wrolpi-navbar').should(($nav) => {
const bar = $nav[0].getBoundingClientRect().height;
const more = [...$nav[0].querySelectorAll('button')]
.find(button => button.textContent.includes('More'));
expect(more, 'the bar has a More button at this width').to.not.be.undefined;
expect(more.querySelector('svg'), 'More has its dropdown caret').to.not.be.null;
expect(more.getBoundingClientRect().height, 'More fills the bar')
.to.be.closeTo(bar, 1);
expect(getComputedStyle(more).cursor, 'More is clickable').to.equal('pointer');
});
});
});
describe('the icon-only triggers in the navbar corner are reachable', () => {
/*
* The search glyph carried Semantic's `item` class, which has had no rules at all since
* Semantic was removed: no pointer cursor, no hover highlight, and a hit area of the
* 18px glyph rather than the height of the bar. The share glyph did compute
* `cursor: pointer` -- it is an `<a href>` -- but its target was the 24px box the icon
* occupies, so the pointer showed only if you found it exactly.
*
* Both are icon-only triggers sitting beside Help and Admin, and should behave as those
* do. Measured against a real link in the same bar rather than against a number.
*/
/*
* The REAL ShareButton, and the real SearchIconButton that DesktopNav renders for
* itself. The first version of this suite used hand-written anchors carrying the
* classes I expected the components to have, so reverting SearchIconButton to Semantic's
* `item` left it green -- it was measuring my markup, not the app's.
*/
const corner = <NavIconWrapper><ShareButton/></NavIconWrapper>;
const triggers = {
share: '.wrolpi-navbar-icon a',
search: '.wrolpi-navbar [aria-label="Search"]',
};
beforeEach(() => cy.mountUI(
<MemoryRouter><div style={{width: 1400}}>
<DesktopNav
navColors={{background: '#5c4fa8', color: '#ffffff', ratio: 6.69}}
homeLink={<a className='wrolpi-navbar-link' href='#'><i>WROLPi</i></a>}
icons={corner}
/>
</div></MemoryRouter>,
));
Object.entries(triggers).forEach(([name, selector]) => {
it(`gives ${name} a pointer and the bar's full height`, () => {
cy.get('.wrolpi-navbar').should(($nav) => {
const bar = $nav[0].getBoundingClientRect().height;
const trigger = $nav[0].querySelector(selector.replace('.wrolpi-navbar ', ''));
expect(trigger, `${name} is in the bar`).to.not.be.null;
expect(getComputedStyle(trigger).cursor, `${name} cursor`).to.equal('pointer');
expect(trigger.getBoundingClientRect().height, `${name} hit area`)
.to.be.closeTo(bar, 1);
});
});
it(`highlights ${name} on hover, as a link does`, () => {
/*
* The highlight is what tells a user the glyph is a control at all, and it is
* the half of the complaint that a cursor check does not cover.
*
* `:hover` cannot be forced -- no event applies it, and getComputedStyle will
* never report it -- so the real stylesheets are searched for a hover rule this
* element matches that paints a background. Resolved against the live CSSOM
* rather than a file, so it fails if the rule is edited, dropped, or written
* with a selector that misses.
*/
cy.get(selector).should(($el) => {
const element = $el[0];
const painted = [...document.styleSheets].flatMap((sheet) => {
try {
return [...sheet.cssRules];
} catch (e) {
return []; // A cross-origin sheet; none of ours are.
}
}).filter(rule => rule.selectorText
&& rule.selectorText.includes(':hover')
&& rule.style.backgroundColor
&& rule.selectorText.split(',').some((selector) => {
const resting = selector.replace(/:hover/g, '').trim();
try {
return resting && element.matches(resting);
} catch (e) {
return false;
}
}));
expect(painted.map(rule => rule.selectorText), `${name} hover rule`)
.to.not.be.empty;
});
});
});
});
describe('the navbar recalculates when the corner changes, not only the window', () => {
/*
* The corner is not a fixed width. The ⌘K hint beside the search glyph renders only
* once `useKeyboardDetected` has seen a keypress, so it appears at an arbitrary later
* moment; the load, memory, temperature and drive warnings appear and vanish as the
* machine's state changes. Each one changes the room the links have, and none of them
* resizes the container.
*
* With only the container observed nothing recalculated, and the bar wrapped -- which is
* why the same viewport wrapped or not depending on whether the user had touched a key,
* and why the width sweep could pass while the real bar was broken. The sweep measures
* a bar whose corner never changes after mount; this one changes it.
*/
const GrowingCorner = () => {
// Stands in for the ⌘K hint and for a warning icon arriving: a corner that is one
// width when the bar is measured and a wider one a moment later.
const [grown, setGrown] = React.useState(false);
React.useEffect(() => {
const timer = setTimeout(() => setGrown(true), 150);
return () => clearTimeout(timer);
}, []);
return <NavIconWrapper>
<span data-testid='corner-filler'
style={{display: 'inline-block', width: grown ? 180 : 20}}/>
</NavIconWrapper>;
};
const mountGrowing = (width) => cy.mountUI(
<MemoryRouter><div style={{width}}>
<DesktopNav
navColors={{background: '#5c4fa8', color: '#ffffff', ratio: 6.69}}
homeLink={<a className='wrolpi-navbar-link' href='#'><i>WROLPi</i></a>}
icons={<GrowingCorner/>}
/>
</div></MemoryRouter>,
);
it('gives up links when the corner grows after the bar was measured', () => {
mountGrowing(900);
// The corner has finished growing...
cy.get('[data-testid="corner-filler"]').should(($filler) => {
expect($filler[0].getBoundingClientRect().width, 'the corner grew').to.equal(180);
});
// ...and the bar is still one row, which it can only be if it recalculated.
cy.get('.wrolpi-navbar').should(($nav) => {
const rows = new Set([...$nav[0].querySelectorAll('.wrolpi-navbar-link')]
.map(link => Math.round(link.getBoundingClientRect().top)));
expect([...rows], 'every link shares one row').to.have.length(1);
});
});
it('takes the links back when the corner shrinks again', () => {
/*
* The inverse, and the reason to recalculate rather than simply reserve the widest
* corner the bar might ever have: a warning icon that clears must give its space
* back, or the bar keeps a permanently shortened set of links until the next resize.
*/
mountGrowing(900);
cy.get('[data-testid="corner-filler"]').should(($filler) => {
expect($filler[0].getBoundingClientRect().width).to.equal(180);
});
cy.get('.wrolpi-navbar').then(($nav) => {
const shrunken = $nav[0].querySelectorAll('.wrolpi-navbar-link').length;
// Shrink the corner back, as a cleared warning would.
cy.get('[data-testid="corner-filler"]').then(($filler) => {
$filler[0].style.width = '20px';
});
cy.get('.wrolpi-navbar').should(($after) => {
expect($after[0].querySelectorAll('.wrolpi-navbar-link').length,
'links come back when the corner clears').to.be.greaterThan(shrunken);
});
});
});
});
describe('the global search modal shows its results', () => {
/*
* The suggestion list overlays the page everywhere else, and should: a list that reflows
* the page underneath while you type is unusable. Inside the search modal that same rule
* failed twice over. An absolutely positioned list contributes no height, so the modal
* panel sized itself to the search box alone -- 114px -- and the panel's own
* `overflow: auto` then clipped the list at that edge. What the user got was a search
* box and the first line of the first group, with the page showing through below.
*
* The real SearchBox, in a panel with the modal's overflow, so the class that carries the
* fix comes from the component rather than from markup written here.
*/
const results = {
channels: {
name: 'Channels',
results: Array.from({length: 8}, (unused, index) => ({
title: `Channel ${index + 1}`, description: 'A channel of videos',
})),
},
domains: {
name: 'Domains',
results: Array.from({length: 8}, (unused, index) => ({
title: `example${index + 1}.com`, description: 'An archived domain',
})),
},
};
// The modal panel: Mantine caps its height and scrolls it, which is what did the clipping.
const panel = (children) => <div className='fake-modal-content'
style={{maxHeight: 504, overflow: 'auto', width: 440}}>
{children}
</div>;
// The list is a combobox: it opens on typing, as it does for a real user.
const openSuggestions = () => cy.get('.wrolpi-searchbox input').type('n');
it('lays the suggestions out inside the panel rather than over the page', () => {
cy.mountUI(panel(<div className='wrolpi-search-modal'>
<SearchBox value='chan' results={results} onResultSelect={() => {}}/>
</div>));
openSuggestions();
cy.get('.wrolpi-searchbox-results').should(($results) => {
expect(getComputedStyle($results[0]).position, 'the list is in flow')
.to.equal('static');
});
});
it('grows the panel to fit them, so nothing is clipped', () => {
cy.mountUI(panel(<div className='wrolpi-search-modal'>
<SearchBox value='chan' results={results} onResultSelect={() => {}}/>
</div>));
openSuggestions();
cy.get('.fake-modal-content').should(($panel) => {
const box = $panel[0].getBoundingClientRect();
const list = $panel[0].querySelector('.wrolpi-searchbox-results')
.getBoundingClientRect();
expect(list.height, 'the list has results to show').to.be.greaterThan(100);
// The whole list is inside the panel, not spilling past its clipped edge.
expect(list.bottom, 'the list ends inside the panel')
.to.be.at.most(box.bottom + 0.5);
// And the panel actually grew for it, rather than the list having been squashed.
expect(box.height, 'the panel grew to hold the list')
.to.be.greaterThan(list.height);
});
});
it('is a real constraint: outside the modal the list still overlays', () => {
/*
* The inverse. If the rule leaked to every searchbox, the file browser and the
* videos page would push their content down on every keystroke -- a worse bug than
* the one being fixed, and one this suite would otherwise not notice.
*/
cy.mountUI(panel(
<SearchBox value='chan' results={results} onResultSelect={() => {}}/>
));
openSuggestions();
cy.get('.wrolpi-searchbox-results').should(($results) => {
expect(getComputedStyle($results[0]).position, 'still an overlay elsewhere')
.to.equal('absolute');
});
});
});

16
app/src/components/ui/ui.css
View file

@ -367,6 +367,22 @@ html[data-theme="amber"] .wrolpi-searchbox-control:focus-within {
border-top: none;
}
/*
* Inside the global search modal the suggestions ARE the panel's content, so they sit in
* flow instead of floating over the page.
*
* Overlaying is right everywhere else -- a list that reflows the page while you type is
* unusable -- but in a modal it fails twice. An absolutely positioned list contributes no
* height, so the panel sized itself to the search box alone (114px), and the panel's own
* `overflow: auto` then clipped the list at that edge: the modal showed an input and the
* first line of the first group. In flow, the panel grows to fit and its own max-height
* does the scrolling.
*/
.wrolpi-search-modal .wrolpi-searchbox-results {
position: static;
max-height: min(50vh, 420px);
}
.wrolpi-searchbox-group-name {
padding: 6px 10px;
font-size: 11px;

31
app/src/hooks/useOverflowNav.js
View file

@ -84,7 +84,26 @@ export function useOverflowNav({links, containerRef, homeRef, rightMenuRef, more
setIsReady(true);
}, []); // eslint-disable-line react-hooks/exhaustive-deps
// ResizeObserver for ongoing resizes.
/*
* Recalculate when the container resizes -- and when either of the two fixed blocks
* either side of the links does, which is the part that was missing.
*
* The corner does not keep a constant width. The ⌘K hint beside the search glyph only
* renders once `useKeyboardDetected` has seen a keypress, so it materialises at an
* arbitrary later moment and widens the corner by about 30px. The status indicators do
* the same in both directions: the load, memory, temperature and drive warnings appear
* and disappear as the machine's state changes, and a Raspberry Pi under load grows two
* or three of them.
*
* Every one of those changes the space the links have, and none of them changes the
* container's size -- so with only the container observed, nothing recalculated and the
* bar simply wrapped. That is the other half of "at certain widths": whether the extra
* width overflowed depended on how much slack the last-fitted link happened to leave, so
* the same viewport wrapped or not depending on whether the user had touched a key.
*
* This converges rather than oscillating: the More button lives outside the corner, so
* the corner's width does not depend on how many links are visible.
*/
useLayoutEffect(() => {
const container = containerRef.current;
if (!container) return;
@ -93,8 +112,16 @@ export function useOverflowNav({links, containerRef, homeRef, rightMenuRef, more
recalculate();
});
observer.observe(container);
if (rightMenuRef.current) {
observer.observe(rightMenuRef.current);
}
// `homeRef` is a `display: contents` span, which has no box of its own to observe.
const homeEl = homeRef.current?.firstElementChild;
if (homeEl) {
observer.observe(homeEl);
}
return () => observer.disconnect();
}, [containerRef, recalculate]);
}, [containerRef, homeRef, rightMenuRef, recalculate]);
const visibleLinks = links.slice(0, visibleCount);
const overflowLinks = links.slice(visibleCount);