mirror of
https://github.com/lrnselfreliance/wrolpi.git
synced 2026-10-26 19:01:39 -04:00
fix: toasts were covering the navigation bar's controls
Six keyboard-shortcuts e2e tests started failing on CI: clicking the search glyph was refused because a `mantine-Notification-title` was on top of it. The toast container is fixed at `top: var(--mantine-spacing-md)` -- 16px -- while the bar occupies the first 54px of the page. Every toast has therefore been landing on the bar's right-hand corner: the search glyph, the hamburger, Help and Admin. For as long as the toast is up those controls cannot be clicked, and an error toast is precisely the moment a user reaches for Admin. It surfaced now because the previous commit gave the search glyph a proper hit area. At 18px it sat in the gap beneath the toast; grown to the height of the bar, it moved into the overlap. The toast has been covering that corner all along -- there was nothing there to hit. So this is not a test that needs loosening: CI found a real defect that predates the change that exposed it. Toasts now start below the bar. `--navbar-height` is shared between the bar's min-height and the offset so the two cannot drift, and the rule matches `[data-position^='top']` so a future move to bottom-right does not silently inherit a top offset. Mantine's own positioning uses `:where()`, which has zero specificity, so a single class wins. Tested by `elementFromPoint` rather than by comparing rectangles: that is the question Cypress's actionability check asks and the one a mouse asks. A geometry test would miss a toast that overlaps but sits behind, and a z-index test would miss one that is in front but elsewhere on screen. Three cases -- the search control, every control in the corner, and the offset itself, which is stated separately because the first two would also pass on a toast that merely happened to be narrow enough to miss at one viewport. Removing the offset reddens all three. Confirmed on the running app with a real error toast up: toast top 70px, bar bottom 54px, all six corner controls topmost at their own centres. The e2e spec passes locally, 12 of 12. 1124 jest / 62 suites, 199 in ui-layout.cy.js, clean npm run build.
This commit is contained in:
parent
ad78cdee7c
commit
29105da2b6
2 changed files with 118 additions and 1 deletions
|
|
@ -1,3 +1,8 @@
|
|||
:root {
|
||||
/* Shared with the toast offset below, so the two cannot drift apart. */
|
||||
--navbar-height: 3.4em;
|
||||
}
|
||||
|
||||
/* Nav bar: a hand-rolled flexbox bar (was a Semantic Menu). The user-chosen background
|
||||
and the foreground measured against it are set inline (see themes/navColors); this
|
||||
stylesheet only supplies structure and hover affordances. */
|
||||
|
|
@ -10,10 +15,30 @@
|
|||
* is the one link a user needs to reach the settings that fix a broken WROLPi.
|
||||
*/
|
||||
flex-wrap: wrap;
|
||||
min-height: 3.4em;
|
||||
min-height: var(--navbar-height);
|
||||
padding: 0 0.25em;
|
||||
}
|
||||
|
||||
/*
|
||||
* Toasts start below the navigation bar instead of on top of it.
|
||||
*
|
||||
* They are fixed at `top: var(--mantine-spacing-md)` -- 16px -- while the bar occupies the
|
||||
* first 54px of the page, so every toast landed over the bar's right-hand corner: the search
|
||||
* glyph, the hamburger, Help and Admin. For as long as the toast was up those controls could
|
||||
* not be clicked, and an error toast is exactly when a user reaches for Admin.
|
||||
*
|
||||
* It went unnoticed until the search glyph was given a proper hit area: at 18px it sat in the
|
||||
* gap under the toast, and growing it to the height of the bar moved it into the overlap.
|
||||
* The toast was always covering that corner; there was just nothing there to hit.
|
||||
*
|
||||
* `:where()` gives Mantine's own rule zero specificity, so one class is enough to win, but
|
||||
* the attribute is matched too -- only the top-anchored positions need moving, and a future
|
||||
* switch to `bottom-right` should not silently inherit a top offset.
|
||||
*/
|
||||
html .mantine-Notifications-root[data-position^='top'] {
|
||||
top: calc(var(--navbar-height) + var(--mantine-spacing-md, 16px));
|
||||
}
|
||||
|
||||
.wrolpi-navbar-right {
|
||||
display: flex;
|
||||
align-items: center;
|
||||
|
|
|
|||
|
|
@ -2745,3 +2745,95 @@ describe('the global search modal shows its results', () => {
|
|||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('a toast does not cover the navigation bar', () => {
|
||||
/*
|
||||
* Toasts are fixed at `top: var(--mantine-spacing-md)` -- 16px -- while the bar occupies
|
||||
* the first 54px of the page, so every toast landed over the bar's right-hand corner:
|
||||
* the search glyph, the hamburger, Help and Admin. For as long as it was up those
|
||||
* controls could not be clicked, and an error toast is exactly the moment a user reaches
|
||||
* for Admin.
|
||||
*
|
||||
* It surfaced as six e2e failures the day the search glyph was given a proper hit area:
|
||||
* at 18px it sat in the gap beneath the toast, and growing it to the height of the bar
|
||||
* moved it into the overlap. The toast had been covering that corner all along; there
|
||||
* was simply nothing there to hit.
|
||||
*
|
||||
* Measured as `elementFromPoint`, which is the same question Cypress's actionability
|
||||
* check asks and the one a mouse asks -- a geometry-only comparison would miss a toast
|
||||
* that overlaps but sits behind, and a z-index comparison would miss one that is in
|
||||
* front but elsewhere on the screen.
|
||||
*/
|
||||
const themeIcon = <NavIconWrapper>
|
||||
<IconButton icon='sun' label='Theme' variant='subtle'/>
|
||||
</NavIconWrapper>;
|
||||
|
||||
const mountBarWithToast = () => {
|
||||
cy.mountUI(<MemoryRouter>
|
||||
<Notifications position='top-right'/>
|
||||
<DesktopNav
|
||||
navColors={{background: '#5c4fa8', color: '#ffffff', ratio: 6.69}}
|
||||
homeLink={<a className='wrolpi-navbar-link' href='#'><i>WROLPi</i></a>}
|
||||
icons={themeIcon}
|
||||
/>
|
||||
</MemoryRouter>);
|
||||
cy.then(() => {
|
||||
clearToasts();
|
||||
// The shape CI produces: an error toast, which is the kind that lingers.
|
||||
toast({type: 'error', title: 'Unexpected error', description: 'Something failed.'});
|
||||
});
|
||||
cy.get('.mantine-Notification-root').should('exist');
|
||||
};
|
||||
|
||||
it('leaves the search control clickable while a toast is up', () => {
|
||||
mountBarWithToast();
|
||||
|
||||
cy.get('.wrolpi-navbar [aria-label="Search"]').should(($search) => {
|
||||
const box = $search[0].getBoundingClientRect();
|
||||
const topmost = document.elementFromPoint(
|
||||
box.left + box.width / 2, box.top + box.height / 2);
|
||||
|
||||
expect(topmost, 'something is at the search glyph').to.not.be.null;
|
||||
expect($search[0].contains(topmost) || topmost === $search[0],
|
||||
`search is the topmost element, not ${topmost && topmost.className}`).to.be.true;
|
||||
});
|
||||
});
|
||||
|
||||
it('leaves every control in the corner clickable', () => {
|
||||
// Help and Admin are the ones that matter most: an error toast is when a user goes
|
||||
// looking for the admin pages, and it was sitting on top of the link.
|
||||
mountBarWithToast();
|
||||
|
||||
cy.get('.wrolpi-navbar-right').should(($corner) => {
|
||||
const controls = [...$corner[0].querySelectorAll('a, button')];
|
||||
expect(controls.length, 'there are controls to check').to.be.at.least(3);
|
||||
|
||||
const covered = controls.filter((control) => {
|
||||
const box = control.getBoundingClientRect();
|
||||
const topmost = document.elementFromPoint(
|
||||
box.left + box.width / 2, box.top + box.height / 2);
|
||||
return !(control.contains(topmost) || topmost === control);
|
||||
}).map(control => control.textContent.trim()
|
||||
|| control.getAttribute('aria-label') || '(unlabelled)');
|
||||
|
||||
expect(covered).to.deep.equal([]);
|
||||
});
|
||||
});
|
||||
|
||||
it('starts the toast below the bar rather than beside it', () => {
|
||||
/*
|
||||
* The mechanism, stated separately from the effect. The two cases above would also
|
||||
* pass if the toast merely happened to be narrow enough to miss the controls at this
|
||||
* viewport, which is how the bar came to be covered at some widths and not others.
|
||||
*/
|
||||
mountBarWithToast();
|
||||
|
||||
cy.get('.mantine-Notifications-root').should(($root) => {
|
||||
const toastTop = $root[0].getBoundingClientRect().top;
|
||||
const barBottom = Cypress.$('.wrolpi-navbar')[0].getBoundingClientRect().bottom;
|
||||
|
||||
expect(barBottom, 'the bar has height to clear').to.be.greaterThan(40);
|
||||
expect(toastTop, 'the toast starts below the bar').to.be.at.least(barBottom);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue