fix(app-shell): every CelPredicateField binds its label — useId fallback when no id is passed (#4533) - #4561
Merged
Conversation
…ack when no id is passed (#4533) CelPredicateField renders BOTH halves of its own label association — the `Label htmlFor` and the `Textarea id` — but `id` was an optional prop with no fallback, so a call site that omitted it left both `undefined`: a label that read correctly on screen and resolved to nothing, and an editor with no accessible name. The RLS `USING (read filter)` and `CHECK (write filter)` clauses on the permission-set editor were mounted that way, so the row-filter authoring surface was unreachable by label. It surfaced from the authoring side: writing objectui#4302's wire pins, `getByLabelText('USING (read filter)')` was refused outright and that test had to select by placeholder instead. Fixed at the component rather than the callers: `id` falls back to `useId()`, so the association is a property of mounting the component instead of every caller remembering the prop. An explicit `id` still wins unchanged. The autocomplete listbox is named off the same resolved id, closing the same association hole on `aria-controls`. Two corrections to the card's table, both measured on origin/main: ConditionalFormattingEditor was listed as omitting `id` but has passed `cf-condition-{i}` since #2558; and inspectors/ConditionBuilder.tsx DOES omit it and was not listed — the component-level fallback repairs it with no call-site edit, which is the argument for fixing the contract. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
|
PM step-7 复核 — ACCEPT (session_017Qqyix2QcnpUC9XeYVDzx3)
Auto-merge armed (squash) — landing verified per the merge-queue discipline. Generated by Claude Code Generated by Claude Code |
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
2 participants
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.
Fixes #4533
CelPredicateFieldrenders both halves of its own label association — theLabel htmlForand theTextarea id— butidwas an optional prop with no fallback, so a call site that omitted it left bothundefined: a label that reads correctly on screen and resolves to nothing, and an editor with no accessible name.The RLS
USING (read filter)andCHECK (write filter)clauses on the permission-set editor were mounted that way, so the row-filter authoring surface of a permission set had no accessible name. It surfaced from the authoring side rather than from review — writing #4302's wire pins, the obvious selector was refused verbatim:The fix is at the contract, not the call sites
idnow falls back toReact.useId(), used for both halves. The association becomes a property of mounting the component instead of every caller remembering a prop, so a future call site cannot silently reintroduce an unlabelled control. An explicitidstill wins unchanged.Per the ruling's second point, no call site is edited. The measurement for "concrete need" — stable ids asserted by existing tests — finds four such assertions, all
document.getElementById('cf-condition-0')inConditionalFormattingEditor.test.tsx, i.e. against a site that already passes an explicit id. Zero existing tests assert an id at either RLS site, so the fallback is the whole fix and the surface stays at one component, one new test, one changeset.One deliberate consequence, pinned rather than left unremarked:
listIdwasid ? id-ac : undefined, so an id-less mount also had noaria-controls— the combobox never announced its own popup. It is derived from the resolved id now, closing the same association hole in the same mechanism.Two corrections to the card's table
Both measured on
origin/mainbefore any edit:PermissionAdvancedFacets.tsxRLS USINGididPermissionAdvancedFacets.tsxRLS CHECKididConditionalFormattingEditor.tsxidcf-condition-{i}inspectors/ConditionBuilder.tsxidObjectFieldInspector.tsx(x4)ididConditionalFormattingEditorhas passed an explicit id since the feature landed —git log -S "cf-condition-"has exactly one commit, the original #2558. At the card's own filing revision, line 272 is the mount's opening tag and line 287 is theidprop; the line number of the mount was recorded as the defect.Meanwhile
inspectors/ConditionBuilder.tsxgenuinely omitsidand was not in the table. It belongs to another in-flight change and is not edited here — the component-level fallback repairs it with no call-site edit, which is the sharpest argument for fixing the contract rather than the callers.Red-first
Predictions were written before the suite ran. The six id-less cases were predicted red and failed with the refusal byte-identical to the one quoted in the card (6/6); the seven explicit-id pins were predicted green on unfixed
mainand were. After the fix: 13/13.Reverse-verified by extracting the fix to a patch and
git checkout --on the component (nevergit stash— the stash stack is shared across worktrees), re-running the final suite against a tree whose component hashed to origin/main's exactsha256:75b1f1b4…, then restoring to a byte-identicalsha256:78f6bbfd….Verification
CelPredicateField,PermissionAdvancedFacets.cel/.retiredKeys,ConditionalFormattingEditor+.celGate,celAuthoring, and fix(app-shell): package-door permission save carries every editor-authorable facet (#4302) #4534'sPermissionMatrixEditor.packageDoorFacets(untouched): 8 files, 101 passedDataPillar.celGate: 47 files, 532 passed, 1 skippedtype-check(both passes:tsc --noEmitandtsc -p tsconfig.test.json): cleanreact-hooks/set-state-in-effecton an effect this PR does not touch, present identically on theorigin/mainbaseline; zero new findingscheck:control-bytes,changeset:check,check:phantom-deps,check:i18n-keys,check:i18n-drift,type-check:coverage: all green.d.tsmeasured both ways withdist/+tsconfig.tsbuildinfocleared between builds: all 415 declaration files byte-identical, andCelPredicateFieldis absent fromdist/index.d.ts— module-local, zero type-surface change, hencepatchMust-not-change, all green:
ObjectFieldInspector's four explicit ids assert verbatim on a real mount;cf-condition-0still resolves; #4534's placeholder-based selection is untouched; lint /onLintChangebehavior unchanged.Generated by Claude Code