getByRole matches accessible names by case-insensitive substring. So { name: 'Distance' } matched:
<buttonclass="cutoffMapToggle">Show this distance on a map</button>
— the map toggle added by the same feature. The assertion was meant to say "no Distance tab in the admissions segmented control" and actually said "no button anywhere whose label contains the word distance".
Confirmed by resolving the locator against staging rather than assuming: it returns exactly that button, and the only element on the page whose text is "Distance" is the sticky-nav <a>, which is a link and was never the match.
Fix
Scoped to the control it is about, via its own aria-label, and read positively:
consttabs=awaitviewToggle.getByRole('button').allTextContents();expect(tabs).toContain('This year');// proves the scope resolved
expect(tabs).not.toContain('Distance');// the actual assertion
An absence check against an unscoped locator passes for the wrong reason the moment the selector stops matching — which is precisely how the regression this test guards would come back unnoticed. Verified live that the scope resolves and sees ["This year", "13-year trend"], so it is not passing on an empty locator.
Two siblings with the same weakness
{ name: 'Check' } — Check is a prefix of the button's own busy label Checking…. Now exact: true.
The figure matcher accepted (miles|m) — a leftover from the mixed-unit era that would have kept passing if the headline regressed to metres, i.e. the exact thing #105 just fixed. Now requires miles.
A real defect behind the second one
Tightening that matcher to /\d+\.\d+ miles\b/ failed — and the reason is not the regex:
textContent: "0.88 miles1.4 km"
The cut-off figure and its metric support are flex children with the gap drawn by CSS and nothing between them in the text layer. So a miles\b boundary can never match, and more to the point a screen reader announces the two figures run together.
Both templates now carry an explicit {' '}. Whitespace text nodes are not rendered as flex items, so the reading changes and the layout does not — and there is a unit test pinning it, since it is cheap to lose again.
Verification
54/54 e2e green against staging (was 53 passed / 1 failed)
208 frontend tests, tsc clean, next build green
The e2e regex change is compatible with the currently deployed markup as well as the new one, so it will not flip red between merge and deploy
CI failure on `a school page shows no year-by-year cut-off record`:
```
Locator: getByRole('button', { name: 'Distance' })
Expected: 0
Received: 1
```
**The test was wrong; the page was correct.**
## Cause
`getByRole` matches accessible names by case-insensitive **substring**. So `{ name: 'Distance' }` matched:
```html
<button class="cutoffMapToggle">Show this distance on a map</button>
```
— the map toggle added by the same feature. The assertion was meant to say *"no Distance tab in the admissions segmented control"* and actually said *"no button anywhere whose label contains the word distance"*.
Confirmed by resolving the locator against staging rather than assuming: it returns exactly that button, and the only element on the page whose text **is** "Distance" is the sticky-nav `<a>`, which is a link and was never the match.
## Fix
Scoped to the control it is about, via its own `aria-label`, and read **positively**:
```ts
const tabs = await viewToggle.getByRole('button').allTextContents();
expect(tabs).toContain('This year'); // proves the scope resolved
expect(tabs).not.toContain('Distance'); // the actual assertion
```
An absence check against an unscoped locator passes for the wrong reason the moment the selector stops matching — which is precisely how the regression this test guards would come back unnoticed. Verified live that the scope resolves and sees `["This year", "13-year trend"]`, so it is not passing on an empty locator.
## Two siblings with the same weakness
- `{ name: 'Check' }` — `Check` is a prefix of the button's own busy label `Checking…`. Now `exact: true`.
- The figure matcher accepted `(miles|m)` — a leftover from the mixed-unit era that would have kept passing if the headline regressed to metres, i.e. the exact thing #105 just fixed. Now requires miles.
## A real defect behind the second one
Tightening that matcher to `/\d+\.\d+ miles\b/` failed — and the reason is not the regex:
```
textContent: "0.88 miles1.4 km"
```
The cut-off figure and its metric support are flex children with the gap drawn by CSS and **nothing between them in the text layer**. So a `miles\b` boundary can never match, and more to the point a screen reader announces the two figures run together.
Both templates now carry an explicit `{' '}`. Whitespace text nodes are not rendered as flex items, so the reading changes and the layout does not — and there is a unit test pinning it, since it is cheap to lose again.
## Verification
- **54/54 e2e green against staging** (was 53 passed / 1 failed)
- 208 frontend tests, `tsc` clean, `next build` green
- The e2e regex change is compatible with the currently deployed markup as well as the new one, so it will not flip red between merge and deploy
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01WDvkyqqHABm4bmth2kjAxE
The failing journey was wrong; the page was correct.
Locator: getByRole('button', { name: 'Distance' })
Expected: 0 Received: 1
getByRole matches accessible names by case-insensitive SUBSTRING, so
{ name: 'Distance' } matched <button>Show this distance on a map</button> —
the map toggle added by the same feature. The assertion was meant to say "no
Distance tab in the admissions segmented control" and instead said "no button
anywhere whose label contains the word distance".
Now scoped to the control it is about, via its own aria-label, and read
positively: the tab list must contain "This year" and must not contain
"Distance". An absence check against an unscoped locator passes for the wrong
reason the moment the selector stops matching, which is exactly how the
regression this test guards would return unnoticed.
Two sibling locators had the same weakness and are tightened: 'Check' is a
prefix of the button's own busy label "Checking…", and the figure matcher
accepted `(miles|m)` — a leftover from the mixed-unit era that would have kept
passing if the headline regressed to metres, which is the thing #105 just
fixed.
Tightening that matcher surfaced a real defect behind it. The cut-off figure
and its metric support are flex children with the gap drawn by CSS and nothing
between them in the text layer, so the element read "0.88 miles1.4 km" —
what a screen reader announces, and why a `miles\b` boundary could never
match. Both templates now carry an explicit space. Whitespace text nodes are
not rendered as flex items, so the reading changes and the layout does not.
Verified against staging: 54/54.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WDvkyqqHABm4bmth2kjAxE
This PR fixes a real accessibility/text bug where the admissions distance tile rendered the mile figure and its metric equivalent as one concatenated string (e.g. "0.88 miles1.4 km") because whitespace-only text nodes between flex children aren't rendered; it adds an explicit space node in both AdmissionsSection.tsx and SecondaryAdmissionsSection.tsx and updates unit/e2e tests to match and to guard against the regression recurring. The change is scoped, well-reasoned, and low risk to production; only a couple of minor test-robustness nits stand out.
🟡 Minor
e2e/tests/journeys.spec.ts: In the 'Distance' tab test, tabs (from allTextContents(), untrimmed) is asserted with .toContain('This year') directly, while the very next assertion trims each entry before comparing. If the tab button's text content has incidental leading/trailing whitespace (common with JSX formatting or icon markup), the untrimmed assertion could fail spuriously even when the UI is correct, while the trimmed assertion would pass — an inconsistency that risks CI flakiness.
e2e/tests/journeys.spec.ts: The distance-figure regex was narrowed from /\d+(\.\d+)? (miles|m)\b/ to /\d+\.\d+ miles/, which now only matches figures with a decimal point. This relies on the app always formatting mile figures with a fractional component; if a future change ever renders a whole-number mile value without a decimal, this assertion would fail even though the UI is correct.
## 🤖 AI Code Review (Claude Code)
This PR fixes a real accessibility/text bug where the admissions distance tile rendered the mile figure and its metric equivalent as one concatenated string (e.g. "0.88 miles1.4 km") because whitespace-only text nodes between flex children aren't rendered; it adds an explicit space node in both AdmissionsSection.tsx and SecondaryAdmissionsSection.tsx and updates unit/e2e tests to match and to guard against the regression recurring. The change is scoped, well-reasoned, and low risk to production; only a couple of minor test-robustness nits stand out.
### 🟡 Minor
- **e2e/tests/journeys.spec.ts**: In the 'Distance' tab test, `tabs` (from allTextContents(), untrimmed) is asserted with `.toContain('This year')` directly, while the very next assertion trims each entry before comparing. If the tab button's text content has incidental leading/trailing whitespace (common with JSX formatting or icon markup), the untrimmed assertion could fail spuriously even when the UI is correct, while the trimmed assertion would pass — an inconsistency that risks CI flakiness.
- **e2e/tests/journeys.spec.ts**: The distance-figure regex was narrowed from `/\d+(\.\d+)? (miles|m)\b/` to `/\d+\.\d+ miles/`, which now only matches figures with a decimal point. This relies on the app always formatting mile figures with a fractional component; if a future change ever renders a whole-number mile value without a decimal, this assertion would fail even though the UI is correct.
tudor
merged commit 8f211577c8 into main2026-08-20 21:18:28 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
CI failure on
a school page shows no year-by-year cut-off record:The test was wrong; the page was correct.
Cause
getByRolematches accessible names by case-insensitive substring. So{ name: 'Distance' }matched:— the map toggle added by the same feature. The assertion was meant to say "no Distance tab in the admissions segmented control" and actually said "no button anywhere whose label contains the word distance".
Confirmed by resolving the locator against staging rather than assuming: it returns exactly that button, and the only element on the page whose text is "Distance" is the sticky-nav
<a>, which is a link and was never the match.Fix
Scoped to the control it is about, via its own
aria-label, and read positively:An absence check against an unscoped locator passes for the wrong reason the moment the selector stops matching — which is precisely how the regression this test guards would come back unnoticed. Verified live that the scope resolves and sees
["This year", "13-year trend"], so it is not passing on an empty locator.Two siblings with the same weakness
{ name: 'Check' }—Checkis a prefix of the button's own busy labelChecking…. Nowexact: true.(miles|m)— a leftover from the mixed-unit era that would have kept passing if the headline regressed to metres, i.e. the exact thing #105 just fixed. Now requires miles.A real defect behind the second one
Tightening that matcher to
/\d+\.\d+ miles\b/failed — and the reason is not the regex:The cut-off figure and its metric support are flex children with the gap drawn by CSS and nothing between them in the text layer. So a
miles\bboundary can never match, and more to the point a screen reader announces the two figures run together.Both templates now carry an explicit
{' '}. Whitespace text nodes are not rendered as flex items, so the reading changes and the layout does not — and there is a unit test pinning it, since it is cheap to lose again.Verification
tscclean,next buildgreen🤖 Generated with Claude Code
https://claude.ai/code/session_01WDvkyqqHABm4bmth2kjAxE
The failing journey was wrong; the page was correct. Locator: getByRole('button', { name: 'Distance' }) Expected: 0 Received: 1 getByRole matches accessible names by case-insensitive SUBSTRING, so { name: 'Distance' } matched <button>Show this distance on a map</button> — the map toggle added by the same feature. The assertion was meant to say "no Distance tab in the admissions segmented control" and instead said "no button anywhere whose label contains the word distance". Now scoped to the control it is about, via its own aria-label, and read positively: the tab list must contain "This year" and must not contain "Distance". An absence check against an unscoped locator passes for the wrong reason the moment the selector stops matching, which is exactly how the regression this test guards would return unnoticed. Two sibling locators had the same weakness and are tightened: 'Check' is a prefix of the button's own busy label "Checking…", and the figure matcher accepted `(miles|m)` — a leftover from the mixed-unit era that would have kept passing if the headline regressed to metres, which is the thing #105 just fixed. Tightening that matcher surfaced a real defect behind it. The cut-off figure and its metric support are flex children with the gap drawn by CSS and nothing between them in the text layer, so the element read "0.88 miles1.4 km" — what a screen reader announces, and why a `miles\b` boundary could never match. Both templates now carry an explicit space. Whitespace text nodes are not rendered as flex items, so the reading changes and the layout does not. Verified against staging: 54/54. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WDvkyqqHABm4bmth2kjAxE🤖 AI Code Review (Claude Code)
This PR fixes a real accessibility/text bug where the admissions distance tile rendered the mile figure and its metric equivalent as one concatenated string (e.g. "0.88 miles1.4 km") because whitespace-only text nodes between flex children aren't rendered; it adds an explicit space node in both AdmissionsSection.tsx and SecondaryAdmissionsSection.tsx and updates unit/e2e tests to match and to guard against the regression recurring. The change is scoped, well-reasoned, and low risk to production; only a couple of minor test-robustness nits stand out.
🟡 Minor
tabs(from allTextContents(), untrimmed) is asserted with.toContain('This year')directly, while the very next assertion trims each entry before comparing. If the tab button's text content has incidental leading/trailing whitespace (common with JSX formatting or icon markup), the untrimmed assertion could fail spuriously even when the UI is correct, while the trimmed assertion would pass — an inconsistency that risks CI flakiness./\d+(\.\d+)? (miles|m)\b/to/\d+\.\d+ miles/, which now only matches figures with a decimal point. This relies on the app always formatting mile figures with a fractional component; if a future change ever renders a whole-number mile value without a decimal, this assertion would fail even though the UI is correct.