fix(cards): bring the last fixed-size parts under cross-axis fit - #330
Merged
Conversation
Change 0042's fourth and final task: the parts the contract reaches that PRs 2 and 3 did not touch, each measured against the content region a 12-column desktop grid at 960px produces — a 63px tile, 35px of region, 19px of it under LCARS. The measurement settled them by three routes, and two of them correct what the change document assumed: - `input_text`'s field and `input_datetime`'s inputs carry 100–200px of inline minimum width and were CLIPPED by the tile rather than merely overhanging it. A field is bounded by its own content on the axis this tier is one cell wide, so it is in the class the contract omits rather than narrows. Both now render no input at `tall`, with the tap moved to `more-info` in the same decision so the tier does not take away the last way to operate the helper. Keyed on the TIER, as PR 2 keyed the stepper: one column is one column, with no arithmetic to underflow. Their inline minimums are gone at every tier as well — 150px overflowed a two-column `row` region (113px) too, which nothing had measured. - The icon circle is 40px against a 35px region, so it overhung into the inset and relied on the padding to stay uncropped. Its arithmetic is not the slider's and is worth recording: a symmetric inset cancels, so the circle is clipped only where the TILE is narrower than it. Now `min(--liebe-icon-circle, 100%)`, with an aspect ratio so a narrowed circle stays circular rather than becoming an ellipse. - The chip and the vertical pill group are recorded as CONFORMING, on the measurement that shows it: their 34px and 38px are `block-size`, and the axis a one-cell-wide tier bounds is the inline one, which a chip takes from its label and a pill group from its container. The change document read both numbers as cross-axis sizes; they are heights. The row line's leftover width stays an open coarse gate — PR 3 recorded it as such and this PR does not close it. Closes change 0042: status flipped in the change document, docs/index.yml and docs/index.md in this commit. Change: docs/changes/0042-tall-tile-control-geometry.md (PR 4)
…fail A mutation probe found the jsdom assertion inert: with the control slot omitted, removing the card's `if (!controlOmitted)` guard renders identically, so the test passed on the mutation it existed to catch. What carries the fallback is `defaultAction="more-info"`, resolved inside the shell's gesture layer — so the assertion moves to the e2e, where a real click opens a real dialog, and the unit file records why it is not there.
…pinned Q6 restored the 150px minimum and every test still passed — the change that fixes the two-column `row` overflow was carried by nothing. Inline styles are in the DOM, so this is one of the few geometry claims jsdom can hold: no px minimum on the readout or the field, and the readout ellipsizes rather than pushing past the tile.
…circle codex review on the first pass, two P2 findings, both valid: - The circle narrowed but its glyph did not. A card's icon is an SVG carrying its own 20-24px dimensions, so a circle clamped to LCARS's 19px region kept a 24px glyph and spilled it over the tint — the overhang moved one box inwards rather than removed, which is the failure mode this change has hit at every level of the chain. - The audit claimed every part was settled while two were still pinned. The person avatar and the media artwork are separate elements in separate sheets, sized from the same token, and they occupy the icon circle's slot — the rule binds the slot, not one occupant of it. Both now take the same min() and aspect ratio, asserted in a test of their own so the three sites cannot drift apart.
…import Local CodeRabbit: the changelog wrote min(--liebe-icon-circle, 100%) without var(), which is invalid CSS in prose that documents CSS, and the new test annotated its helper through the React namespace where its siblings import the type. Its third finding is declined — it asks that PR 4 stay unchecked 'while PR 2 and PR 3 remain open', and both are ticked and merged (#313 and #325); the rest of that finding, flipping the status and syncing both indexes, is what this branch already does.
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 46 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 (3)
📝 WalkthroughWalkthroughTall-tier input controls are omitted and taps route to the detail dialog. Icon and artwork slots now adapt to available width while preserving square geometry. Unit, stylesheet, and end-to-end tests validate control omission, truncation, containment, and interaction behavior. Documentation marks the change complete. ChangesTall-tile geometry and controls
Review process documentation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
🟡 Not ready to approve
Configured tapAction: toggle can become a no-op when controls are omitted, and one of the new unit assertions incorrectly rejects a valid minWidth: 0 style.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR implements PR 4 (the audit) of docs/changes/0042-tall-tile-control-geometry.md, bringing the remaining fixed-size parts under the “cross-axis fit” contract, adds unit/e2e coverage for the new behavior, and closes change 0042 by marking it complete in the change doc and indexes.
Changes:
- Make the icon slot (icon circle + avatar/artwork occupants) cross-axis flexible and clamp icon glyph sizing with the circle.
- Omit
input_text/input_datetimeinline inputs attalland route tile tap tomore-info(with new unit + e2e tests). - Update design-system + input-helper option docs and close change 0042 in
docs/index.*.
File summaries
| File | Description |
|---|---|
| tests/e2e/tall-fixed-parts-fit.spec.ts | New browser-level geometry + interaction assertions for omitted inline inputs and icon fit at tall. |
| src/components/PersonCard/PersonCard.css | Clamp avatar size to the icon-slot rule (min(var(--liebe-icon-circle), 100%), aspect ratio). |
| src/components/MediaPlayerCard/MediaPlayerCard.css | Clamp artwork size to the icon-slot rule (min(var(--liebe-icon-circle), 100%), aspect ratio). |
| src/components/InputTextCard.tsx | Remove fixed min widths; omit controls at tall; default tap to more-info. |
| src/components/InputDateTimeCard.tsx | Remove fixed min widths; omit controls at tall; default tap to more-info. |
| src/components/anatomy/anatomy.css | Make .liebe-icon cross-axis flexible and clamp glyph sizing inside it. |
| src/components/anatomy/tests/anatomyStyles.test.ts | Assert the new icon-circle clamp + glyph scaling rule in the stylesheet test. |
| src/components/tests/inlineControlFit.test.tsx | New unit tests for omission-at-tall and removal of fixed inline minimums. |
| src/components/tests/iconSlotFlexibility.test.ts | New unit test asserting the avatar/artwork icon-slot clamp matches .liebe-icon. |
| src/components/tests/cardTierLayouts.test.tsx | Update tier-layout expectations for input helper cards at tall (no inline controls). |
| docs/specs/entity-cards/options/input-helpers.md | Update tier table + add explicit note about tall omitting embedded inputs. |
| docs/specs/design-system/index.md | Record PR 4’s audit outcomes and mark cross-axis fit as now describing shipped behavior. |
| docs/index.yml | Mark change 0042 as complete. |
| docs/index.md | Mark change 0042 as complete. |
| docs/changes/0042-tall-tile-control-geometry.md | Mark change 0042 as complete and check off PR 4. |
Review details
Comments suppressed due to low confidence (1)
src/components/tests/inlineControlFit.test.tsx:158
- Same issue as the readout check: a correct
minWidth: 0will appear as0pxand should be allowed; this currently rejects any px value including 0.
expect(field.style.minWidth, 'the field must not pin an inline minimum').not.toMatch(/\d+px/)
- Files reviewed: 15/15 changed files
- Comments generated: 4
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #330 +/- ##
=======================================
Coverage 88.36% 88.37%
=======================================
Files 233 233
Lines 8476 8482 +6
Branches 2852 2854 +2
=======================================
+ Hits 7490 7496 +6
Misses 788 788
Partials 198 198 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
There was a problem hiding this comment.
🟡 Not ready to approve
The review found a few concrete issues in the updated/new tests/docs (including an order-dependent store setup in a new unit test) that should be addressed to avoid flakiness and confusion.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Review details
Comments suppressed due to low confidence (3)
docs/specs/entity-cards/options/input-helpers.md:88
- This bullet restates the detailed measurement/rationale that’s already captured in the design-system’s “Cross-axis fit” section, which increases drift risk between the two specs. Consider keeping only the observable rule here and linking to the design-system section for the rationale.
- **`input_text` and `input_datetime` render no embedded input at `tall`, and their tap falls back to `more-info` there.** A one-column tile's content region is 35px on a 12-column desktop grid, against a 100px readout and a 150px field for the text helper and 120/200px for the datetime one — so the tile's own `overflow: hidden` cropped them, and neither can be narrowed to fit, because a field is bounded by its content on exactly the axis this tier is one cell wide ([design-system — cross-axis fit](../../design-system/index.md#cross-axis-fit), change [0042](../../../changes/0042-tall-tile-control-geometry.md) PR 4). The omission is keyed on the **tier**, not on a measured width: `tall` is one column by definition, so the rule needs no arithmetic and holds at 35px, at LCARS's 19px, and where a 16-column screen leaves no content region at all. The two halves are one decision — dropping the input without moving the tap would leave the tile inert, which the floors are not permitted to do.
src/components/tests/inlineControlFit.test.tsx:83
seed()merges new entities into whatever was already inentityStore, so this test’s setup can become order-dependent if a previous test left entities behind. ClearentityStore.entitiesinbeforeEachbefore seeding so the cases run against only the intended fixtures.
beforeEach(() => {
vi.clearAllMocks()
hass = createMockHomeAssistant()
seed(entity('input_text.note', 'hello', { friendly_name: 'Note', mode: 'text' }))
seed(
tests/e2e/tall-fixed-parts-fit.spec.ts:138
- The comment says a “
rowneighbour is placed beside it”, but the seeded neighbour iswidth: 2, height: 2and the test later assertsrow.tieris'full'. This mismatch makes the test intent harder to follow.
* The password helper is the `input_text` fixture the suite already seeds, so
* this measures the card family rather than a fixture invented for it. A
* `row` neighbour is placed beside it: the omission below is about ONE tier,
* and a card that had simply stopped rendering its field would pass every
* assertion on the tall tile and fail on this one.
- Files reviewed: 16/16 changed files
- Comments generated: 0 new
- Review effort level: Low
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
Copilot on #330, three findings, all valid. The important one: `defaultAction="more-info"` covers a tile whose action is `default`, and a tile with an explicit `tapAction: toggle` resolves to the card's own click handler whatever the tier — so with the input omitted it called the handler, found nothing to focus, and did nothing at all. That is the inert tile the floors are not permitted to produce, and it is the same shape as the unavailable-tile gap already on the backlog. Both cards now return `'more-info'`, the escape hatch `GridCard`'s `onClick` contract exists for and that the fan and lock cards already use. Also: the `minWidth` assertion used `/\d+px/`, which matches the `0px` React serializes a zero to — it passes today because the value is empty, but it would have failed a correct `minWidth: 0` the moment that changed. Tightened to forbid a POSITIVE minimum and to print what it found. And the input-helpers bullet restated measurements the design-system spec owns, against REVIEW.md's single-owner rule; it is now a rule plus links.
The jsdom assertion I added for it was worthless, and a probe said so:
removing `return 'more-info'` left every test green. The negative form
("no edit state after a tap") passes on the broken card because with the
input omitted there is nothing to enter edit state either way, and the
positive form cannot be written in jsdom at all — the shell's gesture
layer does not resolve a click into an action there, so it fails on
correct code.
So the claim moves to the e2e, and gets harder while it moves: the tall
text card is now seeded with an explicit `tapAction: toggle`, which is
the route Copilot actually raised. That is NOT the `defaultAction` path —
the gesture resolves to the card's own handler, which must answer
`more-info` rather than doing nothing. The unit file records both
discarded forms and why, so the next reader does not re-add one.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/components/__tests__/cardTierLayouts.test.tsx (1)
475-495: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCover the tall-tier replacement interaction, not just omission.
These tests prove that the controls are absent, but not the other half of the contract: tapping a control-free
talltile must still open the detail dialog. The existing interaction tests coverglanceonly, so a regression in tall-tier tap routing would pass. Add tall-tier cases for both helpers, or parameterize the existing dialog-path tests by tier and assert thatdetail-controlsappears.Also applies to: 570-577
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/__tests__/cardTierLayouts.test.tsx` around lines 475 - 495, Add interaction coverage for control-free tall-tier tiles in the existing dialog-path tests: exercise both helper types, or parameterize those tests by tier, and tap the tall tile to assert that detail-controls appears. Keep the existing glance coverage and verify tall rendering still routes through the detail dialog despite lacking inline controls.Source: Coding guidelines
src/components/__tests__/inlineControlFit.test.tsx (1)
132-169: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winNo equivalent inline-minimum check for
InputDateTimeCard.This test verifies the
flex/minWidthremoval only forInputTextCard.InputDateTimeCard.tsxmakes the identical style change (field:flex: 1, minWidth: 0; readout:minWidth: 0+ ellipsis) at lines 168-176/210-215, but nothing here asserts it — therow/fulltest above (lines 113-130) only checks that the control is present, not its inline style. Since this is a stated goal of PR 4 (the datetime inputs are called out as "a worse measurement" than the text field), consider mirroring this test forInputDateTimeCardto actually verify the fix rather than relying on code coverage from presence checks alone.🧪 Suggested addition
+ it('carries no fixed inline minimum on the datetime control at `row`', () => { + renderCard(<InputDateTimeCard entityId="input_datetime.alarm" tier="row" />) + + const readout = screen.getByText(/2026/).parentElement! + expect(readout.style.minWidth).not.toMatch(/[1-9]\d*px/) + expect(readout.style.textOverflow).toBe('ellipsis') + + fireEvent.click(screen.getByLabelText('Edit value')) + const field = screen.getByLabelText('Value').parentElement! + expect(field.style.minWidth).not.toMatch(/[1-9]\d*px/) + expect(field.style.flex).not.toBe('') + })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/components/__tests__/inlineControlFit.test.tsx` around lines 132 - 169, Add an equivalent inline sizing assertion for InputDateTimeCard alongside the existing InputTextCard test, covering both row readout and edit-field states. Verify the readout has no positive minWidth and uses ellipsis, then verify the editable field has no positive minWidth and a non-empty flex style, using the existing InputDateTimeCard rendering and accessible elements.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@AGENTS.md`:
- Around line 264-269: Update the duplicate-count validation described in
AGENTS.md so it does not subtract both parent duplicate counts independently
when the same repeated row exists in both parents. Compare merged row
multiplicities against the intended keyed union, or otherwise subtract the
shared baseline only once, ensuring a correct merge passes and an extra merged
duplicate is detected.
- Around line 292-295: Update the patch-coverage guidance in AGENTS.md to label
percentages from reruns using --exclude as diagnostic only, not equivalent to
the 100% codecov/patch gate, since excluded specs may cover changed lines.
Require a full-suite rerun after the tree is green before treating coverage as
gate-grade, and state the exclusion alongside any diagnostic percentage.
In `@docs/specs/design-system/index.md`:
- Around line 197-203: Update the anatomy summary’s `.liebe-icon` entry to
describe 40px as its preferred size rather than a fixed circle size, while
noting that it can shrink to fit its available slot. Keep the rest of the
summary unchanged and align its wording with the sizing rule documented in the
surrounding section.
In `@docs/specs/entity-cards/options/input-helpers.md`:
- Around line 84-88: Update the Primary action bullets for input_text and
input_datetime to include the tall-tier fallback when their embedded controls
are omitted, matching the generic omission wording used by input_number. Keep
the existing glance more-info behavior and ensure explicit tapAction: toggle
also resolves to more-info at tall.
---
Nitpick comments:
In `@src/components/__tests__/cardTierLayouts.test.tsx`:
- Around line 475-495: Add interaction coverage for control-free tall-tier tiles
in the existing dialog-path tests: exercise both helper types, or parameterize
those tests by tier, and tap the tall tile to assert that detail-controls
appears. Keep the existing glance coverage and verify tall rendering still
routes through the detail dialog despite lacking inline controls.
In `@src/components/__tests__/inlineControlFit.test.tsx`:
- Around line 132-169: Add an equivalent inline sizing assertion for
InputDateTimeCard alongside the existing InputTextCard test, covering both row
readout and edit-field states. Verify the readout has no positive minWidth and
uses ellipsis, then verify the editable field has no positive minWidth and a
non-empty flex style, using the existing InputDateTimeCard rendering and
accessible elements.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 439769f9-c505-486e-a2db-f9d6880bdab6
📒 Files selected for processing (16)
AGENTS.mddocs/changes/0042-tall-tile-control-geometry.mddocs/index.mddocs/index.ymldocs/specs/design-system/index.mddocs/specs/entity-cards/options/input-helpers.mdsrc/components/InputDateTimeCard.tsxsrc/components/InputTextCard.tsxsrc/components/MediaPlayerCard/MediaPlayerCard.csssrc/components/PersonCard/PersonCard.csssrc/components/__tests__/cardTierLayouts.test.tsxsrc/components/__tests__/iconSlotFlexibility.test.tssrc/components/__tests__/inlineControlFit.test.tsxsrc/components/anatomy/__tests__/anatomyStyles.test.tssrc/components/anatomy/anatomy.csstests/e2e/tall-fixed-parts-fit.spec.ts
CodeRabbit on #330, four findings, all valid. The one that matters: the note I had just written said to subtract "each parent's own duplicate count", and that is wrong arithmetic. A baseline both parents share is ONE baseline — subtracting it twice scores a correct merge at -1 and lets a genuinely duplicated row (three headers where the parents had two) score 0. The rule is to compare each row's merged multiplicity against the HIGHEST either parent already had. Worth noting that the script I actually ran did it correctly; only the prose was wrong, which is the same prose-drifts-from-tool shape the note is about. Also: an excluded-spec coverage figure is diagnostic and not gate-grade, since the excluded spec may cover changed lines — it needs a full-suite rerun once the tree is green before it means anything. The anatomy summary still called the icon circle a fixed "40px circle" three lines above the section that repins it as a preferred size. And the primary action bullets for both helpers still read as glance-only, where input_number's was already generalised to "any tier where the control was omitted" for exactly this reason.
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 4 of
docs/changes/0042-tall-tile-control-geometry.md— the change's last task — and therefore closes 0042:**Status:** completein the change document,docs/index.ymlanddocs/index.md, all in the implementing commit.PR 4 is the audit: the parts the cross-axis-fit contract reaches that PRs 2 and 3 did not touch. Each was measured against the content region a 12-column desktop grid at 960px produces — a 63px tile, 35px of region, 19px of it under LCARS — and the measurement corrects two of the change document's own assumptions.
What the measurement found, and the three routes it settled them by
Omitted, because they cannot be sized down at all.
input_text's field (100px readout, 150px edit box) andinput_datetime's inputs (120/200px — the widest fixed inline sizes on any card) were clipped by the tile rather than merely overhanging it, which puts them alongside the stepper PR 2 removed rather than the slider PR 3 narrowed. A field is bounded by its own content on exactly the axis this tier is one cell wide, so it is in the class the contract omits. Both now render no input attall, and the tap falls back tomore-infoin the same decision — dropping the input alone would leave the tile inert, which the floors are explicitly not permitted to do. Keyed on the tier, as PR 2 keyed the stepper: one column is one column, with no arithmetic to underflow at 35px, at 19px, or at the 16-column case with no region at all.Their inline minimums are gone at every tier as well. 150px overflowed a two-column
rowregion (113px on the same grid) too — a clip at a tier nobody had measured, found while measuring this one.Made cross-axis flexible. The icon circle is 40px against a 35px region. Its arithmetic is not the slider's and is worth stating: a symmetric inset cancels, so the circle is clipped only where the tile itself is narrower than it (a 16-column screen at 900px lays out a 39px tile) and merely overhangs its region at every width between — which is still the reliance on the inset the first rule forbids. Now
min(var(--liebe-icon-circle), 100%)with an aspect ratio, so a narrowed circle stays a circle rather than becoming an ellipse.Recorded as conforming, on the measurement that shows it. The chip and the vertical pill group were listed as 34px and 38px parts. Both numbers are
block-size. The axis a one-cell-wide tier bounds is the inline one, and on that axis a chip ispadding-inlineplus its label and a pill group is a grid taking its width from the container. Neither carries a fixed cross-axis size, so neither can overhang a region it is sized from. That is a different statement from being outside the rule, and the spec now says which.Review found two more parts, and they are in
codex review --base mainraised two P2s, both valid and both fixed here:An inert tile the review caught, and how it relates to the open one
Copilot found that with the input omitted, a configured
tapAction: toggledid nothing at all:defaultAction="more-info"only covers a tile whose action isdefault, so an explicit toggle resolved to the card's own handler, found nothing to focus, and returned. Atglancethe tap is the only affordance, so that is the operability regression the design system forbids — introduced by this PR attall, and pre-existing atglance.It is the same user-visible symptom as the tracked unavailable-tile item, and a different mechanism — which is why this one is fixable here and that one is not. That item's cause is in
performDispatch: atoggleroute returns early while the entity isunavailable, before it would consult the card'sonToggle, so the card never gets asked and cannot answer. This one is reached:useCardActionshonours'more-info'returned from the toggle handler, exactly as 0037 PR 2's capability gate does, so both cards now answer with it. Worth recording, because the next person to see "tap does nothing on an inoperable tile" will otherwise assume one cause and go looking in the wrong layer.The assertion for it lives in the e2e and nowhere else, and that was settled by two discarded probes rather than by preference. The negative jsdom form ("no edit state after a tap") passes on the broken handler — with the input omitted nothing enters edit state either way. The positive jsdom form cannot be written at all, because the shell's gesture layer does not resolve a click into an action in that environment.
tests/e2e/tall-fixed-parts-fit.spec.tsseeds the tall tile with an explicittapAction: toggleand asserts the dialog opens, which is the one place the claim can fail.What this PR does not close
The row line's leftover width remains a coarse gate rather than an exact capacity, exactly as PR 3 recorded it. Nothing here changes that and the spec paragraph making the capacity claim still says so.
Tests
inlineControlFit.test.tsx— the omission attall, the inputs kept atrow/full, and the flexibility (no px minimum, readout ellipsizes) that a probe caught as unpinned.iconSlotFlexibility.test.ts— the avatar and the artwork held to the circle's rule.anatomyStyles.test.ts— the circle's clamp, its aspect ratio, and the glyph clamp.cardTierLayouts.test.tsx— the twotallrows updated to the new contract, with the reason in the test.tests/e2e/tall-fixed-parts-fit.spec.ts— new: the premise (region narrower than the circle) asserted rather than assumed, then no control slot attall, the icon inside its region and still square, a real tap opening the real dialog, and the neighbour proving the token is what a roomy tier still measures.One assertion was deliberately not written in jsdom. A probe showed the tap guard is inert there — with the slot gone, removing the card's guard renders identically — so the fallback is asserted in the browser instead, and the unit file records why it is absent rather than leaving a green that establishes nothing.
Gates
npm test(6662),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 on the merged tree. Patch coverage checked on both zero-hitDA:lines and untakenBRDA:branches for every changed source line: clean.status: completecounts 36 on main and 37 here — exactly the one change this PR closes.Mutation-probed: 12 landed, 12 caught. Two passing probes were discarded rather than scored — one inert (the tap guard, moved to e2e), one a real gap (the field's flexibility, now pinned and re-probed red). Local CodeRabbit: 3 findings, 2 applied, 1 declined (it asked that PR 4 stay unchecked "while PR 2 and PR 3 remain open"; both are ticked and merged as #313 and #325).
Summary by CodeRabbit
New Features
Documentation
Tests