Skip to content

fix(frontend): correct seven small UI defects - #126

Merged
pdevito3 merged 8 commits into
mainfrom
fix/ui-small-defects
Oct 25, 2026
Merged

pdevito3 merged 8 commits into
mainfrom
fix/ui-small-defects

Conversation

@pdevito3

Copy link
Copy Markdown
Owner

Why

A UI pass found seven small faults in the SPA. Each fault has its own commit, and that commit also holds the functional test of the fault.

What changed

1. Short pages scroll

Cause. The Home and About routes and the component library set min-h-screen on their wrappers. The wrapper is below the 64 px header and above the 16 px bottom padding, and the desktop inset adds 8 px above and below. So a short page was one screen plus 96 px (80 px on a phone). Fix. The wrappers do not set a height. The layout already fills the screen. Proof. The scroll range of /about is 96 at 1280x800 and 80 at 390x844 before, 0 after.

Before After

2. An unknown address shows a bare "Not Found"

Cause. The router had no not-found component, so TanStack Router showed its default text, logged a notFoundComponent warning two times, and the breadcrumb showed "Page". Fix. NotFound is the defaultNotFoundComponent of the router. It has a heading and a link to the home page. The breadcrumb shows "Not Found" when a match is the global not-found match. Proof. 0 links before, 1 after, and no warning.

Before After

3. The skeletons pulse with reduced motion

Cause. Skeleton used animate-pulse. Fix. The skeleton uses motion-safe:animate-pulse. The weather loading grid is now a status named "Loading forecast", as the loading filter control already is. Proof. With reduced motion, 18 animations run in the loading grid before and 0 after. Without it, they still pulse. A still image cannot show this, so there is no screenshot.

The motion.dev warning "You have Reduced Motion enabled on your device" stays. Motion logs it only in a development build, each time it honors the setting, and the only way to remove it is to ignore the setting. A production build (vite preview) logs it 0 times on the pages where the dev server logs it 3 times, and the sidebar indicator still jumps in place of a slide.

4. The toast region has no focus ring

Cause. F6 moves the focus to the toast region, which had outline-none and no other focus style. Fix. The region has the focus ring of a button. The ring of a focused toast now also matches the button (3 px at 50 percent, not 2 px at full color). Proof. The focused region has box-shadow: none before and a 3 px ring after.

Before After

5. Stacked toasts cover a tall bottom sheet

Cause. While a bottom sheet is open on a phone, the toasts move to the top. At 667x375 the date filter sheet starts 56 px from the top, and three toasts covered it down to 176 px. Fix. In that state, index.css hides each toast after the newest one visually (React Aria lists the newest first). The hidden toasts stay in the accessibility tree, and a toast shows while the focus is in it. Proof. 3 toasts on the screen before, 1 after, and 3 in the accessibility tree.

Before After

6. The button sizes do not line up

Cause. The row of button sizes on /components did not center its items, so the 28, 32, 36, and 32 px buttons lined up at the top. Fix. The row has items-center. The other demo rows have items of one height. Proof. The centers are up to 4 px apart before and 0 after.

Before After

7. The boolean filter repeats its name

Cause. In the filter dialog, the title "Add Is Blocked Filter" names the property, and the boolean control showed "Is Blocked" again. Fix. The label is visually hidden in the dialog and stays in the desktop submenu. The True and False buttons are in a group that the label names through aria-labelledby, so a screen reader reads the name once. This is the gap that #120 left out of scope. Proof. At 390x844 the label takes up space in the dialog before and is 1 px after. At 1280x800 the submenu shows it before and after.

Before After

Scope

  • Covers the seven faults above in frontend/src, and a functional test for each one except the button row.
  • Leaves the motion.dev development warning as it is (see 3).

Blast Radius

The not-found page and the toast rule touch every page. The toast rule applies only below 768 px while a bottom sheet is open. The other changes are local to one component or one route.

Verification

  • New and changed specs in frontend/tests/features/ (app-shell, toasts, weather, filter-demo). A small helper, takesUpSpace in frontend/tests/support/screen.ts, tells a visually hidden element (1 px, as sr-only makes it) from a shown one, because Playwright counts a 1 px element as visible. On main they fail on the assertions above (scroll range 96 and 80, no not-found heading, 18 animations, no focus ring, 3 toasts shown, label shown). With the fixes, 33 pass, and 330 pass with --repeat-each=10.
  • pnpm lint, pnpm build, pnpm test (80 passed), and pnpm test:e2e (33 passed). Each commit type-checks on its own.
  • Screenshots in light and dark at 1280x800, 390x844, and 667x375: the full set.

The Home and About routes and the component library set min-h-screen on
their wrappers. The wrapper is below the 64 px header and above the 16 px
bottom padding of the layout, and on a desktop the inset also has an
8 px margin above and below. So each page was one screen high plus 96 px
(80 px on a phone) and scrolled when its content was short. The layout
already fills the screen, so the wrappers do not set a height any more.
The router had no not-found component. For an address that no route
matches, it showed the bare "Not Found" text of TanStack Router, with no
link out, and it logged a warning two times. The breadcrumb showed
"Page". The router now uses NotFound as its default not-found component.
The page has a heading and a link to the home page, and the breadcrumb
shows "Not Found" when the root match is the global not-found match.
Skeleton used animate-pulse, so the loading placeholders pulsed for a
user who asks the system for reduced motion. It now uses
motion-safe:animate-pulse. The loading grid of the weather page is now a
status named "Loading forecast", as the loading filter control already
is, so a screen reader and the test can find it.

The motion.dev warning about reduced motion stays in development. Motion
logs it only in a development build, whenever it honors the setting, and
the only way to remove it is to ignore the setting.
F6 moves the focus to the toast region, but the region had outline-none
and no other focus style, so a keyboard user could not see where the
focus was. The region now shows the focus ring of a button. The ring of
a focused toast was 2 px in the full ring color. It now also matches
the button: 3 px at 50 percent, with the ring color on the border.
While a bottom sheet is open on a phone, the toasts move to the top of
the screen. On a phone on its side (667x375), a tall sheet such as the
date filter starts 56 px from the top, and three stacked toasts covered
its title and its first controls. In that state only the newest toast
now shows. React Aria lists the newest toast first, so the rule hides
each later toast visually. The hidden toasts stay in the accessibility
tree, and a toast shows again while the focus is in it.
The row of button sizes on the component page is a flex row with the
default align-items, so the 28, 32, 36, and 32 px buttons lined up at
the top and their centers were up to 4 px apart. The row now centers
its items. The other demo rows have items of one height and do not
change.
In the filter dialog, the title "Add Is Blocked Filter" already names the
property, and the boolean control showed "Is Blocked" again below it. The
label is now visually hidden in the dialog and stays visible in the
desktop submenu, where it is the only name. The True and False buttons
are now in a group that the label names, so a screen reader gives them
a name in both places and reads the name once.
@pdevito3

Copy link
Copy Markdown
Owner Author

Review gate result: no high or medium findings. One low finding is fixed in this PR: the boolean filter group used aria-label, so a screen reader read the name twice. The group now uses aria-labelledby and points at its label. Three low findings stay open:

  1. Older toasts can time out unseen (frontend/src/index.css, the toast rule in the width < 48rem block). When a bottom sheet is open on a phone, the rule hides each older toast visually, and its timer keeps running. A sighted user sees an older toast only if they press F6 and Tab to it. This is the intended trade, because the sheet stays readable. A "+N more" count is a possible follow-up.
  2. The region focus ring can be wider than the toasts (frontend/src/components/toaster.tsx). Below md, the region spans inset-x-4, and a toast is at most 500 px wide. From about 532 px to 767 px wide, the F6 ring outlines a box that is wider than the centered toast. Only the look is affected.
  3. The controlLabel locator uses getByText (frontend/tests/features/filter-demo/filter-demo.locators.ts). docs/frontend/testing.md prefers role, label, then data-testid. The existing aboutText locator also uses getByText, and no role selects a visually hidden label.

@pdevito3

Copy link
Copy Markdown
Owner Author

Correction to the earlier comment about the boolean filter: aria-labelledby gives the group the same accessible name that aria-label gave. The visually hidden label text is still in the group, so in browse mode a screen reader can still read "Is Blocked" twice. The markup is the standard group pattern and stays as it is.

Fix-forward in the last commit: the toast region is now centered and at most 500px wide below 768px, so its F6 focus ring fits the toast (it was 635px around a 500px toast at 667x375). The unknown gradient and text classes on the /components wrapper are removed: they pointed to tokens that do not exist.

@pdevito3
pdevito3 merged commit f031443 into main Oct 25, 2026
16 checks passed
@pdevito3
pdevito3 deleted the fix/ui-small-defects branch October 25, 2026 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant