fix(cards): narrow the tall vertical slider to the tile it sits in - #325
Merged
Conversation
The `tall` tier is one column wide by definition, and on a 12-column desktop grid at 960px that column is a 63px tile with a 35px content region — against a 42px track. The slider hung 3.5px into the tile's inset on each side, which the default theme absorbs and LCARS does not (0.5px of clearance on its inline-end side, less under a smaller inset). Change 0042 PR 1 settled the contract: `--liebe-control-height` names the thickness the slider PREFERS, the region bounds it, and a control under either floor is omitted rather than rendered. This applies it across the light, cover, fan and `input_number` tall layouts: - the band is capped at the body's width, the band's slot carries the token as its definite width and shrinks with the band, and the track, Radix's positioning wrapper and the thumb each take 100% of the slot capped at the token. The definite width sits on the slot deliberately — every box below it sizes as a percentage, and an auto-width slot around content asking for 100% of it resolves to zero; - below 24px across, or 44px along, the control is omitted from the DOM; - the long-axis capacity is a published signal rather than arithmetic: `CardBody` observes the control band through the same shared observation the shell's content width comes from. The tile's height less the inset, the icon circle, the meta block and the gaps is not derivable from tier and span, and a card may not measure the DOM. The band stays in the DOM when its control is omitted, which is what keeps the measurement from oscillating. `--liebe-card-padding` is untouched, and `row` and `full` are unchanged. Change: docs/changes/0042-tall-tile-control-geometry.md (PR 3)
Codex review on the first pass: `--liebe-control-height` is public theming API, so a theme may pin the track under the 24px floor inside a region that is comfortably over it — and a floor checked against the REGION lets that render, against the spec's requirement that both floors be measured on the control as it renders. The band now carries `min(--liebe-control-height, 100%)` and the boxes below it read 100% of the band, so the band IS the rendered track: one definite width at the head of the chain, and one box whose observation answers both floors. It keeps that width when its control is omitted (`data-band-axis` from the requested orientation, not the body's survivor-only stamp), or an omitted control could never return on a tile that had grown. Scoped to the vertical band, so the climate stepper's and the sensor sparkline's bands render exactly as they did — those are PR 4's.
CodeRabbit, locally: the token-catalogue assertion now forbids a weakened rewording (both 'maximum' and the narrowing), the design system's 'parts that do not hold this rule yet' paragraph says the stepper is omitted rather than made flexible, one comment still named the hook by its old name, and the e2e synchronises the input_number card on its band rather than on the card alone. Its fifth finding is declined: the ResizeObserver deletion it asks to wrap in a finally is already restored by the describe's afterEach, which runs whether or not the assertion throws.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 22 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (17)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #325 +/- ##
==========================================
+ Coverage 88.34% 88.36% +0.02%
==========================================
Files 233 233
Lines 8461 8476 +15
Branches 2847 2852 +5
==========================================
+ Hits 7475 7490 +15
Misses 788 788
Partials 198 198 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR implements change 0042 PR 3 by making the tall tier’s vertical slider cross-axis flexible (narrowing to the tile’s content region instead of always using --liebe-control-height) and omitting the slider under the 24px (cross-axis) and 44px (long-axis) floors. It also establishes the long-axis capacity signal by observing the tall control band and adds/updates unit + e2e coverage and supporting documentation/token catalogue text.
Changes:
- Make the
tallcontrol band publish both axes via sharedResizeObserverplumbing, and use the band’s measured size to apply both omission floors for vertical controls. - Update
CardBody/CSS to size thetallvertical-slider band asmin(--liebe-control-height, 100%)and ensure slider/track/thumb follow the band width. - Add a new e2e spec for tall-slider geometry + floors and update existing e2e/unit tests and docs/token catalogue to reflect the repinned token meaning.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/e2e/tall-stepper-fit.spec.ts | Updates spec commentary to reflect PR 3’s responsibilities vs PR 2/PR 4. |
| tests/e2e/tall-slider-fit.spec.ts | New e2e coverage for tall vertical slider width-fitting + omission floors (cross-axis + long-axis). |
| tests/e2e/slider-fill-geometry.spec.ts | Updates tall slider geometry assertions to reflect “fit to region” behavior. |
| src/theme/tokens.ts | Updates token catalogue description for --liebe-control-height to document “maximum, narrowed to fit”. |
| src/theme/tests/tokens.test.ts | Asserts the token catalogue documents control height as a maximum narrowed by the tile. |
| src/store/inputHelperOptions.ts | Updates inline rationale text to reflect the new flexible slider behavior and omission floors. |
| src/components/InputNumberCard.tsx | Passes controlOrientation at tall so the body can size/measure the band correctly. |
| src/components/cardContentWidth.ts | Generalizes shared observation from width-only to a full content-box size (inline+block), keeping a width wrapper for shells. |
| src/components/CardBody.tsx | Adds band observation + uses band size to apply both omission floors for the tall vertical control; keeps band present when control is omitted. |
| src/components/CardBody.css | Implements tall-band cross-axis fitting (min(token, 100%)) and makes slider/track/thumb follow band width. |
| src/components/tests/inputHelperControlStyle.test.tsx | Verifies input_number declares control axis only at tall. |
| src/components/tests/cardContentWidth.test.tsx | Extends observer seam tests to validate both axes in both RO entry shapes + missing-RO behavior. |
| src/components/tests/cardBodyStyles.test.ts | Adds/updates CSS-declaration assertions for tall band width/selector scoping + track/thumb relaxation. |
| src/components/tests/cardBodyControlFit.test.tsx | Expands floor tests to cover tall vertical floor checks using the observed band box (both axes) and band persistence. |
| docs/specs/design-system/index.md | Updates the cross-axis-fit narrative to reflect PR 3 landing and documents the new long-axis capacity signal. |
| docs/changes/0042-tall-tile-control-geometry.md | Marks PR 3 task as complete in the change doc task list. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Copilot on #325, both threads. The observation module's JSDoc still told a caller it would learn its "width" would never arrive, and its reset export was still named for a width-only instrument — while one shared ResizeObserver now serves the shell's content width and the tall band's box. Swept the file rather than the one line: `resetContentWidthObserver` is now `resetContentBoxObserver`, and its own test file says what the second contract reading the instrument is. The e2e spec dereferenced `track` and `thumb` through `!` with no prior existence assertion, so a DOM-structure change would have failed with a TypeError naming a property rather than saying which part went missing — in the one spec whose job is to establish what only a browser can. All four boxes are now asserted together, before any is read. Also records plainly, in the spec paragraph that claims the capacity signals are established, that a vertical control on a ROW LINE is still bounded by a coarse gate — so "established" is not read as covering it. Refs: docs/changes/0042-tall-tile-control-geometry.md (PR 3)
fx
added a commit
that referenced
this pull request
Jul 30, 2026
Two things this change's PRs paid for, both of which read as a check that ran when it did not. **The patch check has no input on a red tree.** Vitest with the v8 provider writes no coverage/lcov.info when the run fails and clears the previous one, so a patch-coverage check taken while anything is failing reads a stale file or nothing — and "nothing", intersected with the diff, yields no uncovered lines, which is a pass. It bites exactly when main is red, which is when a branch is most likely to be re-verified. The workaround is to rerun excluding the failing spec and to state the exclusion alongside the number, since the exclusion is invisible in the figure. **A repeated column header is not the separator case.** Item 9's rule excludes rows made of dashes; `| Token | Default | Purpose |` is ordinary words, and it appears twice in BOTH parents because the design-system spec has two token tables — so the merged file carries a duplicate the merge did not introduce. Two agents hit it independently on that one file within an hour (#325, #329), each reading it as a real duplicate to remove before tracing it, which is item 9's own warning that the false positive is the dangerous direction. The correction is the threshold rather than another exclusion: no duplicate the merge INTRODUCED, subtracting each parent's own.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Implements PR 3 only of
docs/changes/0042-tall-tile-control-geometry.md— "Make the vertical slider cross-axis flexible" — across the light, cover, fan andinput_numbertalllayouts. The change document staysdraft: PR 4 remains, and neitherdocs/index.ymlnordocs/index.mdis touched.The
talltier is one column wide by definition. On a 12-column desktop grid at 960px that column is a 63px tile with a 35px content region, against a 42px track — so the slider hung 3.5px into the tile's inset on each side. The default theme absorbs that and LCARS does not (0.5px of clearance on its inline-end side), which is why PR 1 ruled out simply accepting it.What the fix is
--liebe-control-heightwas re-pinned by PR 1 as the thickness the slider prefers. This applies that:min(--liebe-control-height, 100%)and the slot, the track, Radix's positioning wrapper and the thumb each read 100% of it. The definite width sits at the head of the chain deliberately — every box below it sizes as a percentage, and a fit-content box around content asking for 100% of it resolves to zero, which is the failure 0034 PR 1 measured at 0 against the token's 42 and which no stylesheet assertion can see.--liebe-control-heightis public theming API, so a theme may pin a 10px track inside a 35px region.tallbands render exactly as they did — those parts are PR 4's.--liebe-card-paddingis untouched, androwandfullare unchanged.The long-axis capacity signal
PR 3 owns establishing it, and it is established rather than deferred. The band's height is the tile's height less the inset, the icon circle, the meta block and the gaps — none of which a card knows, and no arithmetic on tier and span recovers it.
CardBodyobserves the band through the same sharedResizeObserverthe shell's content width already uses, and publishes it to the control decision; the prohibition it respects is that a card never measures the DOM, and this is the design system's own layout component measuring a box the cards do not own. The design-system spec records the signal and its ownership.The band stays in the DOM, with its width, when its control is omitted (
data-band-axiscomes from the requested orientation, not the body's survivor-only stamp). Without that the measurement is an oscillator in one direction and a one-way door in the other: a band removed with its control reports "unobserved" and the control returns, and a band that lost its width reports 0 and the control never returns however wide the tile grows.Tests
cardBodyStyles.test.ts— the sheet's declarations, including that no other band's width is touched.cardBodyControlFit.test.tsx— both floors, the theme-pinned-token case, band persistence, and the axis hook.cardContentWidth.test.tsx— the shared observation reports both axes in either entry shape.tokens.test.ts— the catalogue states the token is a maximum narrowed to fit a tile.tests/e2e/tall-slider-fit.spec.ts— new, three browser-level measurements at a recorded container width and span: the track taking the region rather than the token (light andinput_number, thumb included), no slider under LCARS's 19px region, and no slider where the inset leaves a 1×2 band under 44px.slider-fill-geometry.spec.ts's thickness assertion is flipped to the region — it pinned the old behaviour.Nothing local can see a zero-width control, so the browser-level assertions are the gate; CI runs them.
Gates
npm test(254 files, 5955 tests),npm run lint(0 errors, 1 pre-existingMigrationCutoffswarning),npm run typecheck,npm run build-storybookandnpm run test:coverageall pass, each read from its own exit status. Patch coverage checked on both zero-hitDA:lines and untakenBRDA:branches for every changed source line: clean.Mutation-probed: 13 probes, 12 caught on the first run. The one that passed was discarded rather than scored — it was the
data-band-axiswiring, whose effect only a browser sees, so an assertion pinning the hook was added and the probe re-run red.Pre-PR review:
codex review --base mainraised the themed-token gap above (P2), which is fixed here; local CodeRabbit raised five minor findings, four applied and one declined (it asked for afinallyaround aResizeObserverdeletion that the describe'safterEachalready restores).