Repository navigation
feat(apollo-react): select and move through rules in the decision text [ECS-2140] - #1551
Conversation
📊 Coverage + size by packagePer-package coverage and bundle size on this PR. New-line coverage = of the source lines this PR adds or changes, the % hit by tests.
"Coverage" is each package's own |
0fa58db to
bcd2298
Compare
pateljay43
changed the title
Dependency License Review
License distribution
Excluded packages
|
There was a problem hiding this comment.
🟡 Changes recommended
Outstanding issues cause scroll jumps in oversized rules, leave selected chips clipped after resizing, and produce incorrect rule accessible names.
3 open issues
What changed in this PR
Extends the sentence-based DecisionCard introduced in #1543 with controlled selection and keyboard navigation, aligning it with DecisionTable.
Changes:
- Adds rule and cell selection, keyboard navigation, focus handling, and selection styling.
- Preserves selected rows during virtualization and exposes scrolling, container access, and host overlays.
- Adds interaction tests, accessible cell labels, and a selection story.
| File | Description |
|---|---|
packages/apollo-react/src/decision-table/useSentenceNavigation.ts |
Implements keyboard navigation, focus, and selected-cell reveal. |
packages/apollo-react/src/decision-table/useGridNavigation.ts |
Exports shared navigation constants and helpers. |
packages/apollo-react/src/decision-table/sentenceTypes.ts |
Defines selection, ref handle, and cell-label contracts. |
packages/apollo-react/src/decision-table/sentenceStyles.ts |
Adds selection rings and revealed-text styling. |
packages/apollo-react/src/decision-table/sentenceMarks.ts |
Exports value-column ordering for navigation. |
packages/apollo-react/src/decision-table/RuleCard.tsx |
Adds focusable stops, cell labels, and selection indicators. |
packages/apollo-react/src/decision-table/DecisionCard.tsx |
Connects selection, scrolling, ref methods, and overlays. |
packages/apollo-react/src/decision-table/DecisionCard.test.tsx |
Covers selection, navigation, focus, scrolling, and ref behavior. |
packages/apollo-react/src/decision-table/DecisionCard.stories.tsx |
Demonstrates controlled selection and reports selected state. |
packages/apollo-react/src/decision-table/__fixtures__/sentences.fixtures.ts |
Supplies accessible cell-label text. |
Review effort: Balanced
pateljay43
changed the title
bcd2298 to
7b1260e
Compare
bda56cf to
ccf19df
Compare
…t [ECS-2140] A host's selected rule or cell is a keyboard stop that Tab lands on; arrows, Home and End move it, a click selects without scrolling, and focus follows only selections the host applies. Lines wrap instead of clipping, and the window lays rules out at their measured heights.
Storybook visual diffBaseline is the deployed main Storybook, so changes merged to main after this branch was last updated can also appear here. Logs Updated (PT): Nov 13, 2026, 11:57:36 AM |
alucauipath
left a comment
alucauipath
left a comment
There was a problem hiding this comment.
Approving: the roving focus, retained selected row and add-rule focus handling look solid and well tested. A few things worth addressing (the first one before or right after merge):
1. Rule cards lost their run verdicts in the accessible name. ruleCardModel.ts replaces the card's aria-labelledby (heading + lead, which included the lead's sr-only "Matched"/"Not matched" spans) with an aria-label built from shownLineText(heading) + shownLineText(lead), which has no verdicts. For rules worded entirely in the lead (e.g. the grid story's rules with lines: []), focusing the rule no longer announces whether the run matched its conditions. Cell stops still carry their verdicts via verdictOf, so only the rule stop regressed. Could the lead's verdicts be added to the label, or the card go back to aria-labelledby?
2. Scroll jumps in Safari (worth a manual check). With lines wrapping, measured card heights replace estimates as cards mount. When an overscanned card above the viewport measures taller than its estimate, content below shifts; Chrome/Firefox compensate with scroll anchoring, Safari doesn't. Worth trying a scroll-up through long wrapped rules at a narrow width in Safari.
3. containerProps.onKeyDown is skipped for keys the view claims (useContainerHandlers.ts), and since the view runs first, a host can't preventDefault to keep a key. Probably intended, but the containerProps doc still says its handlers "run after the view's"; worth updating.
4. Refocus-on-removal relies on blur. The per-render effect in useContainerHandlers.ts refocuses the view when focus is lost while focusWithinRef is set. Browsers that don't fire blur when the focused element is removed may behave differently from jsdom here; worth checking by deleting the focused selected row in a real browser.
5. Nits: the measured heights map in useRuleWindow.ts never drops row ids that are gone; and since a new rules identity now also rebuilds offsets and re-measures every render, the rules prop doc could say to memoize it, as labels does.
Adds selection and keyboard to
DecisionText: a rule or one of its cells can be selected (selected/onSelect, the same{ rowId, columnId }shape the grid selects), Tab lands on the selected stop, and the arrow keys, Home and End move it, ignoring keys while an IME composes, like the grid. The view keeps the selected row mounted while windowing, scrolls to a selection the host moves, and leaves a clicked rule where it is. Long lines wrap instead of clipping, and a focused add-rule action stays in view after the rule it adds. ECS-2140Stacked on #1543.