diff --git a/app/src/App.css b/app/src/App.css
index dde63798..6f5a98df 100644
--- a/app/src/App.css
+++ b/app/src/App.css
@@ -71,6 +71,44 @@ html .wrolpi-navbar .mantine-ActionIcon-root:hover {
color: inherit;
}
+/*
+ * One status indicator's slot. Laid out rather than nudged: see NavIconWrapper in Nav.js
+ * for the 0.8em top margin this replaces and why it looked correct on half the indicators.
+ */
+.wrolpi-navbar-icon {
+ display: flex;
+ align-items: center;
+ margin-left: 1.5em;
+}
+
+/*
+ * An anchor in the corner hugs its glyph instead of a text line box.
+ *
+ * A line box reserves descender space for text that is not there, which leaves an icon
+ * sitting about 3px above the centre of the box being centred -- the share and search
+ * glyphs, but not the hotspot or theme picker, which are square ActionIcons that centre
+ * their own glyph. That difference is what made a single top margin look right on some
+ * indicators and wrong on others.
+ */
+.wrolpi-navbar-right a {
+ display: flex;
+ align-items: center;
+}
+
+/*
+ * A link in the corner fills the bar's height, as one outside it does.
+ *
+ * Help and Admin highlighted only a box around their text while Videos and Statistics
+ * highlighted the full height. Same class and same rules; the difference is the parent.
+ * The bar stretches its children, but the corner is a nested flex container that centres
+ * its own, so a link inside it was sized to its content. The hover background is painted
+ * on the link box, so this is the hover affordance itself and not just its outline.
+ */
+.wrolpi-navbar-right .wrolpi-navbar-link,
+.wrolpi-navbar-right .wrolpi-navbar-link-button {
+ align-self: stretch;
+}
+
.channel-edit {
border-radius: 3px;
padding: 0.75em;
diff --git a/app/src/components/Nav.js b/app/src/components/Nav.js
index d0a78733..9baba2da 100644
--- a/app/src/components/Nav.js
+++ b/app/src/components/Nav.js
@@ -102,15 +102,30 @@ function DropdownMenuItem({link}) {
}
+/*
+ * The trigger for a navbar dropdown.
+ *
+ * Shared with the hidden placeholder DesktopNav measures on its first render, and that is
+ * the point of extracting it. The placeholder used to be a plain ``
+ * with no caret, so the space reserved for the overflow menu was about 21px short of the
+ * 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) =>
+
+);
+NavDropdownTrigger.displayName = 'NavDropdownTrigger';
+
function DropdownLinks({link}) {
// A labelled dropdown trigger: the desktop "More" overflow, or any nested
// link group handed to MenuLink.
return
}
-function NavIconWrapper({children}) {
+/*
+ * One status indicator's slot in the right-hand corner.
+ *
+ * This used to carry `marginTop: 0.8em`, a nudge left over from Semantic's menu. The corner
+ * centres its children, and a top margin is part of the margin box being centred, so the
+ * nudge pushed every indicator down by half of it -- about 6px.
+ *
+ * That read as an error on some and not others, which is why it survived: the bare-icon
+ * anchors (share, search) sit high inside their line box because it reserves descender space
+ * for text that is not there, so the nudge happened to cancel out. An IconButton is a square
+ * ActionIcon that centres its glyph exactly, so for the hotspot and the theme picker the
+ * nudge was 6px of pure error. Laying the slot out as a flex box centres both shapes on
+ * their real glyph box and needs no nudge at all.
+ */
+export function NavIconWrapper({children}) {
if (children) {
- return
{children}
+ return
{children}
} else {
// Do not use navbar space if children is empty.
return
@@ -383,7 +412,7 @@ export function NavBar() {
}
-function DesktopNav({navColors, homeLink, icons}) {
+export function DesktopNav({navColors, homeLink, icons}) {
const containerRef = React.useRef(null);
const homeRef = React.useRef(null);
const rightMenuRef = React.useRef(null);
@@ -410,7 +439,7 @@ function DesktopNav({navColors, homeLink, icons}) {
))}
{!isReady &&
-
+ }
{isReady && overflowLinks.length > 0 &&
}
diff --git a/app/src/components/ui/ui-layout.cy.js b/app/src/components/ui/ui-layout.cy.js
index 996a8ec5..9ac32c43 100644
--- a/app/src/components/ui/ui-layout.cy.js
+++ b/app/src/components/ui/ui-layout.cy.js
@@ -12,6 +12,7 @@ import {clearToasts, toast} from './toast';
import {monochromeThemes, themeNames} from '../../themes/names';
import {navColorNames} from '../../themes/navColors';
import {NavBarSample} from '../ThemeSamplePage';
+import {DesktopNav, NavIconWrapper} from '../Nav';
/* `--panel` is authored as a hex; computed backgrounds come back as rgb(). */
const hexToRgb = (hex) => {
@@ -2218,3 +2219,187 @@ describe('the navigation bar is legible on every colour a user can pick', () =>
});
});
});
+
+describe('the navigation bar corner', () => {
+ /*
+ * The real , which takes its colours, home link and indicators as props and
+ * so needs none of the polling hooks wires up. The indicators are the shapes
+ * the app actually puts there, in the real NavIconWrapper: a bare-icon anchor (share), an
+ * unwrapped anchor (search), and two IconButtons (hotspot, theme picker). Those three
+ * shapes take their geometry by three different routes, which is the whole problem.
+ */
+ const indicators = <>
+
+ {/* eslint-disable-next-line jsx-a11y/anchor-is-valid */}
+
+
+
+
+ } label='Hotspot'>
+
+ }/>
+
+
+
+
+ ;
+
+ const mountBar = (width, theme = 'light') => cy.mountUI(
+
+ WROLPi}
+ icons={indicators}
+ />
+
,
+ {theme},
+ );
+
+ const centreOf = (rect) => rect.top + rect.height / 2;
+
+ it('centres every indicator on the bar, whatever shape it is', () => {
+ /*
+ * The bug this replaces was a 0.8em top margin on the slot, which the corner centres
+ * along with its content and so half-applied as a ~6px downward shift. It looked
+ * correct on the anchors and wrong on the two IconButtons, because an anchor's glyph
+ * already sits high inside a line box that reserves descender space.
+ *
+ * Measured against the bar's own centre rather than against each other: three things
+ * can be equally and consistently wrong, and were.
+ */
+ mountBar(1400);
+
+ cy.get('.wrolpi-navbar').should(($nav) => {
+ const bar = centreOf($nav[0].getBoundingClientRect());
+ ['share', 'hotspot', 'theme'].forEach((name) => {
+ const glyph = $nav[0].querySelector(`[data-testid="${name}"] svg`);
+ expect(glyph, `${name} has a glyph`).to.not.be.null;
+ expect(centreOf(glyph.getBoundingClientRect()), `${name} vertical centre`)
+ .to.be.closeTo(bar, 1.5);
+ });
+ });
+ });
+
+ it('gives the right-hand links the same hit area as the left-hand ones', () => {
+ /*
+ * Help and Admin highlighted only a box around their text, while Videos and
+ * Statistics highlighted the full height of the bar. The corner is a nested flex
+ * container that centres its children, so a link inside it is sized to its content
+ * while a link that is a direct child of the bar is stretched by `align-items:
+ * stretch`. Same class, same rules, different parent.
+ *
+ * The hover background is painted on the link box, so this IS the hover affordance.
+ */
+ mountBar(1400);
+
+ cy.get('.wrolpi-navbar').should(($nav) => {
+ const bar = $nav[0].getBoundingClientRect().height;
+ const links = [...$nav[0].querySelectorAll('.wrolpi-navbar-link')];
+ const corner = [...$nav[0]
+ .querySelectorAll('.wrolpi-navbar-right .wrolpi-navbar-link')];
+
+ // Help and Admin. If the corner has no links this passes on an empty list.
+ expect(corner.length, 'links in the corner').to.be.at.least(2);
+ expect(links.length, 'links in total').to.be.greaterThan(corner.length);
+
+ corner.forEach((link) => {
+ expect(link.getBoundingClientRect().height, `${link.textContent} hit area`)
+ .to.be.closeTo(bar, 1);
+ });
+ });
+ });
+
+ it('is a real constraint: the left-hand links already filled the bar', () => {
+ // Without this the case above could pass by the bar having collapsed to the height
+ // of its text, which would be a different bug wearing the same number.
+ mountBar(1400);
+
+ cy.get('.wrolpi-navbar').should(($nav) => {
+ const bar = $nav[0].getBoundingClientRect();
+ const first = $nav[0].querySelector('.wrolpi-navbar-link');
+
+ expect(bar.height, 'the bar is taller than a line of text').to.be.greaterThan(40);
+ expect(first.getBoundingClientRect().height, 'a left-hand link fills it')
+ .to.be.closeTo(bar.height, 1);
+ });
+ });
+});
+
+describe('the navigation bar never breaks onto a second line', () => {
+ /*
+ * The bar wraps as a failsafe (`flex-wrap: wrap`) and useOverflowNav is supposed to make
+ * that unreachable by moving links into "More" before they overflow. At certain widths
+ * it did not, and the bar took two rows.
+ *
+ * The arithmetic was off by the bar's own horizontal padding: `recalculate` measured the
+ * WRAPPER div's offsetWidth and subtracted only the home link and the corner, so it
+ * believed it had ~8px more room than the row actually has. Any width where the last
+ * link overhangs by less than that got shown and wrapped -- a narrow band, which is why
+ * it appeared only "at certain widths" while resizing.
+ *
+ * Swept rather than spot-checked, because a band that narrow is precisely what a
+ * spot-check steps over.
+ */
+ /*
+ * Hoisted, not written inline into `icons={...}`. The icon-name audit in ui.test.js
+ * reads every string literal inside such an expression as an icon name, so an inline
+ * `label='Theme' variant='subtle'` fails that test with two names nobody wrote.
+ */
+ const themeIcon =
+
+ ;
+
+ const mountAt = (width) => cy.mountUI(
+
+ WROLPi}
+ icons={themeIcon}
+ />
+
,
+ );
+
+ // Wide enough that every link fits, so this is the height of exactly one row.
+ let singleRow;
+
+ before(() => {
+ mountAt(1600);
+ cy.get('.wrolpi-navbar').then(($nav) => {
+ singleRow = $nav[0].getBoundingClientRect().height;
+ expect(singleRow, 'a single row has a height to compare against')
+ .to.be.greaterThan(30);
+ });
+ });
+
+ /*
+ * 6px steps across the range where links start moving into More. Fine enough to land
+ * inside an 8px band wherever it falls.
+ */
+ const widths = [];
+ for (let width = 1180; width >= 640; width -= 6) widths.push(width);
+
+ it('stays one row at every width from 1180 down to 640', () => {
+ widths.forEach((width) => {
+ mountAt(width);
+ cy.get('.wrolpi-navbar').should(($nav) => {
+ expect($nav[0].getBoundingClientRect().height, `bar height at ${width}px`)
+ .to.be.at.most(singleRow + 2);
+ });
+ });
+ });
+
+ it('keeps every visible link inside the bar rather than clipping it', () => {
+ // The failure mode a naive "just use nowrap" fix would introduce: no second row,
+ // but Admin hanging off the right edge where nothing can reach it.
+ [1180, 1000, 860, 720, 640].forEach((width) => {
+ mountAt(width);
+ cy.get('.wrolpi-navbar').should(($nav) => {
+ const bar = $nav[0].getBoundingClientRect();
+ [...$nav[0].querySelectorAll('.wrolpi-navbar-link')].forEach((link) => {
+ expect(link.getBoundingClientRect().right, `${link.textContent} at ${width}px`)
+ .to.be.at.most(bar.right + 0.5);
+ });
+ });
+ });
+ });
+});
diff --git a/app/src/hooks/useOverflowNav.js b/app/src/hooks/useOverflowNav.js
index 0061f936..c2b5cdc0 100644
--- a/app/src/hooks/useOverflowNav.js
+++ b/app/src/hooks/useOverflowNav.js
@@ -15,7 +15,25 @@ export function useOverflowNav({links, containerRef, homeRef, rightMenuRef, more
const container = containerRef.current;
if (!container || itemWidths.current.length === 0) return;
- const containerWidth = container.offsetWidth;
+ /*
+ * The width a row of links actually has, which is NOT the wrapper's.
+ *
+ * The bar carries `padding: 0 0.25em`, and this measured the wrapper and subtracted
+ * only the home link and the corner -- so it believed it had about 8px more room
+ * than the row does. Any width where the last link overhangs by less than that was
+ * kept in the bar, the bar wrapped, and the user got two rows. A narrow band, which
+ * is why it showed up only "at certain widths" while resizing rather than always.
+ *
+ * SAFETY covers subpixel rounding on top: item widths come from
+ * getBoundingClientRect and are fractional, while clientWidth is an integer.
+ */
+ const SAFETY = 1;
+ const bar = container.firstElementChild || container;
+ const barStyle = window.getComputedStyle(bar);
+ const containerWidth = bar.clientWidth
+ - (parseFloat(barStyle.paddingLeft) || 0)
+ - (parseFloat(barStyle.paddingRight) || 0)
+ - SAFETY;
const homeEl = homeRef.current?.firstElementChild || homeRef.current;
const homeWidth = homeEl ? homeEl.getBoundingClientRect().width : 0;
const rightWidth = rightMenuRef.current ? rightMenuRef.current.getBoundingClientRect().width : 0;