Skip to content

fix(webkit): bring the calendar onto the shared field contract - #827

Open
gabriel-lisboa-azion wants to merge 15 commits into
mainfrom
fix/calendar-field-parity
Open

fix(webkit): bring the calendar onto the shared field contract#827
gabriel-lisboa-azion wants to merge 15 commits into
mainfrom
fix/calendar-field-parity

Conversation

@gabriel-lisboa-azion

@gabriel-lisboa-azion gabriel-lisboa-azion commented Jul 29, 2026

Copy link
Copy Markdown
Collaborator

Summary

Calendar's field surfaces had drifted from the inputs they sit beside. Placed next to a text field, a Calendar did not read as the same control:

  • values rendered at .text-body-sm where every other field uses .text-label-sm;
  • disabled was an opacity wash instead of --bg-disabled / --text-disabled / cursor-not-allowed;
  • hover put a background on whichever segment the pointer was over, so a single-bordered control lit up in pieces.

Hover now raises only the one shared border to --border-strong, through the same selector input-text uses, suppressed while focused (the ring is the active affordance) or disabled. hover:bg-* survives only on the preset menu rows, which are menu items inside the popover, not field surfaces. Preset and period rows take the menu-row height and shape.

Fields are named. Start/End/Date were bare <span>s, leaving each input named only by a placeholder that vanishes on the first keystroke. They are now <label for> bound with useId(), and each time field carries its own aria-label (Start time / End time / Time).

The month strip is restructured so only the days move. The month label and the prev/next controls sit outside the transition, so the control the user just pressed stays put and keeps focus while the month travels.

The focus-ring offset became a variable, not a class fight. The fields sit on the popover's raised surface, so their ring gap has to match it — but input-text hard-coded var(--bg-canvas). It now resolves var(--input-ring-offset, var(--bg-canvas)), and the fields container declares that property once; custom properties inherit, so one declaration covers all four fields and nothing has to out-specify anything.

Tests

Full webkit suite green (84 files / 1710 passing, 6 skipped), eslint + vue-tsc clean, catalog:check in sync. New coverage:

  • the three date/time fields resolve by getByLabelText in both single and range mode, with distinct ids;
  • axe over the open popover with time fields and the timezone selector shown;
  • the month controls survive a month change as the same DOM node and keep focus (the claim the restructure exists to make true).

The axe guard

This branch also adds expectNoPlaceholderOnlyLabels, which runs inside expectNoA11yViolations — so every suite already calling it gets the check.

axe cannot catch a placeholder-only field: placeholder is a valid accname fallback per HTML-AAM, so the label rule passes, and label-title-only covers only title / aria-describedby. Empirically both a bare placeholder-only input and the <span>Start</span> + placeholder shape produce zero axe violations. It is still a defect — the name disappears mid-interaction, and a visible span that reads as a label is associated with nothing.

The guard ships here rather than on its own branch because running it across the suite is what surfaced these Calendar fields: on main it fails on 4 real defects, all of them fixed above. A guard that cannot go green by itself is not a separate PR. It is tested in both directions (13 cases) — a guard that never fires reads as coverage without being any.

input-text / input-number / input-password fixtures gain an aria-label: those components are the bare field, whose label belongs to the wrapping form-field layer, so supplying the name the wrapper would is what keeps the render realistic instead of weakening the assertion to pass.

Visual regression

Baselines are regenerated and committed (ubuntu-latest, via the app-storybook-generate-baseline dispatch). Delta: 64 files, all Calendar, no orphans.

  • 60 moved — the closed trigger: its value drops from .text-body-sm to .text-label-sm and the per-segment hover background is gone.
  • 4 new — the Open story at desktop and tablet.

⚠️ Why an Open story had to be added

All ten existing Calendar stories snapshot the closed trigger. WithTime is a 256px pill reading Oct 8 – 19 and nothing more. So the popover internals — the labelled Start/End fields, the field ring offset, the preset row heights, the month strip, the timezone selector, the chevron rotation — had never been snapshotted, which is most of what this branch changes. Of the 60 moved baselines, all 60 reflect only the trigger's own typography and hover.

The new Open story pins the popover via the controlled open prop. Previewing it locally (darwin, gitignored) before spending a CI run caught two things:

  • the panel was being clipped — the capture crops to #storybook-root's box, and an unsized root shrink-wraps to the 256px trigger, slicing the time fields and timezone chevron out of frame. Fixed with an explicit w-[560px].
  • mobile cannot hold it — the panel is wider than the 375px capture box, so this story shoots desktop + tablet only (visual.modes). A capture limit, per the documented exception; the ten closed-trigger stories still cover all six modes.

Reviewed in both themes: in light the panel reads as a white card with the range band in grey and the endpoints inverted, and each field carries the same border and height as a standalone input-text — the parity this branch set out to deliver, now actually visible. CI renders the timezone as UTC +00:00 rather than a local offset, which is stable because the runner is always UTC.

Notes

  • Merge fix/tab-view-panel-translate first. The month strip renders through the shared panel transition preset; without that fix the slide offset is not interpolated and the swap still reads as a fade. This branch is correct and green either way — the order only decides whether the animation lands complete.
  • The first push of this branch failed the bundle-budget gate; force-pushed with a cheaper fix. The ring-offset override was originally done by moving input-text's root class composition to cn. That pulls tailwind-merge into the most widely imported field primitive in the package: input-text went 1.45 KB → 11.43 KB against a 2 KB budget, and calendar-root 15.57 → 26.42 KB against 17 KB. Budgets only ratchet down, so the right move was a cheaper mechanism rather than a raised ceiling — hence the inherited custom property. Now input-text is 1.46 KB and calendar-root 16.18 KB, both inside budget, with the +0.6 KB being the actual feature work. History was rewritten rather than patched on top, so no commit here adds cn only for a later one to remove it.
  • Default input-text rendering is unchanged, and all six InputText consumers were run (137 tests) plus the full suite.
  • No new dependency. No breaking change.

Review follow-ups (2026-07-29)

A review pass over this branch confirmed the approach and surfaced four small corrections, now applied:

  • The Open story's "Show code" snippet is self-contained. It built its source with toSfc(IMPORT, …) while the template binds v-model="range", so the pasted SFC referenced an undeclared range. It now routes through sfcScript(…) like every sibling story, declaring the same range the canvas renders.
  • --input-ring-offset is documented where it lives. The variable is input-text's public contract, but only the calendar spec and a code comment described it. .specs/input-text.md now carries it in the Tokens table and the a11y section, with the checksum recomputed.
  • Dropped the unused startTimeId / endTimeId from calendar-fields. The time fields are named by aria-label; nothing referenced those ids.
  • The placeholder guard scans exactly the container it is given. The ownerDocument fallback for non-HTMLElement containers silently widened the scan to the whole page; querySelectorAll exists on every Element, so the branch is gone. Its docblocks now state the piggyback on expectNoA11yViolations and the accname subset it checks (an aria-labelledby target is credited by its textContent only), with the dom-accessibility-api upgrade path if that false positive ever bites.

Validated: the two affected suites green in browser mode (52 tests), validate-story-source.mjs --all OK (87 files), eslint + prettier clean on the touched files. The merge-order note above is settled — fix/tab-view-panel-translate (#826) is merged and already an ancestor of this branch.

Docs-page regression in the Open story — fixed

The pinned-open Open story leaked its panel onto the autodocs page: the popover teleports to body with position: fixed coordinates measured once at mount, and on the docs page (every story inline in one document) that measurement happens while the page is still laying out — so the panel froze ~3000px away from its own trigger and floated over the Default story. Clicking Default's input then opened its own (correct) popover beside the orphan, reading as a duplicated calendar. The story canvas and the visual baselines were never affected — the bug only exists in inline docs rendering, which is why every gate passed.

Fixed with the same treatment Dropdown's forced-open stories already use: docs.story = { inline: false, iframeHeight: '720px' }, so on the docs page the story renders in its own iframe and the panel's mount-time geometry is its real geometry. Verified with Playwright against the running docs page: zero leaked panels in the docs document, the panel anchored 47px below its own trigger inside the iframe, and Default's popover opening next to its trigger and closing on Escape with no orphans. (The underlying usePlacement not tracking scroll/resize is pre-existing component behavior, noted for a possible follow-up outside this PR.)

The focus ring's offset colour must match the surface BEHIND the field, but it
was hard-coded to var(--bg-canvas). A field placed on anything else - a raised
popover, a drawer - drew its ring gap in the wrong colour, and the only way out
was for the consumer to pass a competing ring-offset utility and hope it won on
source order.

It now resolves var(--input-ring-offset, var(--bg-canvas)), so a host retunes it
by declaring one custom property on any ancestor. Custom properties inherit, so
a single declaration covers a whole group of fields, and nothing has to
out-specify anything.

Deliberately NOT solved with cn/tailwind-merge: input-text is the most widely
imported field primitive here, and pulling tailwind-merge into it took the entry
from 1.45 KB to 11.43 KB against a 2 KB budget (and calendar-root from 15.6 to
26.4 against 17). Budgets ratchet down, so the fix is the cheaper mechanism, not
a raised ceiling. Default rendering is unchanged.
Calendar's field surfaces had drifted from the inputs they sit beside: values
rendered at .text-body-sm where a text field uses .text-label-sm, disabled was
an opacity wash instead of the disabled background/text tokens, and hover put a
background on whichever segment the pointer was over - so a single-bordered
control lit up in pieces. Hover now raises only the shared border, through the
same selector input-text uses, and is suppressed while focused or disabled.
Preset and period rows take the menu-row height and shape.

Fields are labelled: Start/End/Date were bare spans, leaving each input named
only by a placeholder that vanishes on the first keystroke. They are <label for>
bound with useId(), and each time field carries its own aria-label. The fields
container declares --input-ring-offset once so every field's ring gap matches
the raised popover instead of the canvas.

The month strip is restructured so only the days move: the label and the
prev/next controls sit outside the transition, so the control the user pressed
stays put and keeps focus while the month travels. The slide itself needs the
shared panel-transition preset to interpolate translate (separate PR) - without
it the swap still reads as a fade.
axe cannot catch this: placeholder is a valid accname fallback per HTML-AAM, so
the label rule passes on a placeholder-only field, and label-title-only covers
only title / aria-describedby. Empirically both a bare placeholder-only input
and the '<span>Start</span> + placeholder' shape produce zero axe violations.

It is still a defect - the placeholder disappears on the first keystroke, so
the field loses its name mid-interaction, and a visible span that reads as a
label is associated with nothing. expectNoPlaceholderOnlyLabels runs inside
expectNoA11yViolations, so every suite already calling it gets the check.

It is tested in both directions: a guard that never fires reads as coverage
without being any. Running it across the suite is what surfaced the Calendar
fields this branch labels.
InputText, InputNumber and InputPassword are the bare field - the label belongs
to the wrapping form-field layer, which binds one with useId(). Their fixtures
passed only a placeholder, so under the new guard they read as unnamed fields.

Supplying aria-label is what the wrapper would do, so the render stays
realistic rather than being weakened to pass.
All ten existing stories render the CLOSED trigger, so nothing inside the
popover had ever been snapshotted - the Start/End fields and their labels, the
month strip, the preset rows and the timezone selector were invisible to the
visual layer. That is most of what this branch changes, so the field-contract
work had almost no coverage: only the trigger's own typography and hover.

The Open story pins the popover via the controlled open prop. Two details the
first local run forced: the wrapper needs an explicit WIDTH (the capture crops to
#storybook-root's box, and an unsized root shrink-wraps to the 256px trigger,
slicing the time fields and timezone chevron out of frame), and the story shoots
desktop + tablet only, because the panel is wider than the 375px mobile capture
box. That is a capture limit, not a dodged mode - the ten closed-trigger stories
still cover all six.

The test asserts the fixture really mounts the panel and that each field resolves
by its visible label, so a fixture that stops opening fails a test instead of
quietly shooting an empty canvas.
@robsongajunior
robsongajunior force-pushed the fix/calendar-field-parity branch from 0c548a9 to 322d394 Compare July 29, 2026 15:13
robsongajunior
robsongajunior previously approved these changes Jul 29, 2026
robsongajunior
robsongajunior previously approved these changes Jul 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

4 participants