[FIR-292] Rewrite ff-select on a portaled CDK overlay panel#52
Merged
Conversation
Replace the local position:absolute dropdown with a CDK-overlay panel connected to the trigger (bottom/top fallback, width matched, closes on outside pointer events and repositions on scroll), so it renders above any clipping ancestor such as a modal. Focus is kept on the local trigger/search at all times (never on the portaled options), using aria-activedescendant to track the active option, so the panel does not fight a surrounding cdkTrapFocus. Adds multi-selection (checkboxes, values/valuesChange), bindLabel/bindValue for arbitrary option objects, custom option/selected-label templates, Home/End and typeahead keyboard support. The existing value/valueChange and CVA contract stay unchanged for single-selection callers.
…plates Document the additive inputs/outputs (values, multiple, bindLabel, bindValue, valuesChange), the option/label template slots, and the expanded keyboard/aria guarantees (Home/End, typeahead, activedescendant, aria-multiselectable) without touching the primitive/pattern inventory counts.
…lect catalog Cover the new ff-select capabilities in the playground: multi-selection, bindLabel/bindValue against a plain object array, custom option/label templates, and a searchable select hosted inside an AlertService custom dialog to demonstrate the overlay panel working inside a cdkTrapFocus-guarded ff-dialog-container.
Contributor
Author
|
✅ Verdict: APPROVED (reviewer: mgarcia). All five acceptance criteria met: portaled CDK overlay with correct positioning and close behaviour inside ff-dialog (focus stays trapped via aria-activedescendant), option/label templates + multi-selection + search on top of an unchanged CVA and public API, full keyboard and listbox ARIA, contract + catalog + specs. Doctrine review found no blockers; the CDK overlay CSS consolidation is tracked as FIR-315. Base was 7 commits behind (FIR-288's merge) and both PRs touch the design-system barrel — branch updated by merging develop into the head and CI re-ran green on the updated head. Merging to develop. |
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
1 participant
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.
What
Rewrites the
ff-selectprimitive's options panel on CDK Overlay (portaled to body, connected to the trigger with bottom→top fallback, width-matched, reposition-on-scroll) and extends it — fully additively — with multi-selection (values/valuesChange),bindLabel/bindValuefor arbitrary option objects, custom option/selected-label templates, and complete keyboard support (Home/End, typeahead), keeping the existingvalue/valueChange/CVA contract intact.Why
FF-CAT-06 — the highest-risk P0 migration blocker: 87 hub-select usages in Flydocs, 9 of them
appendTo bodyinside modals. Jira: FIR-292Key decisions
aria-activedescendanton the local trigger/search control (WAI-ARIA listbox pattern), so acdkTrapFocusancestor (ff-dialog-container) never sees focus leave its subtree. Integration specs cover open/Escape inside a trapped dialog.ViewEncapsulation.None),z-indextokenized asvar(--ff-sys-z-dropdown, 1000); consolidating it as a DS-level asset is tracked as debt.select.contract.tsupdated with the new inputs/outputs, template-slot clauses and keyboard/ARIA guarantees; SelectContract stays a primitive (inventory unchanged).How to verify
pnpm nx run-many -t build lint test -p design-system design-system-contract playground— design-system 490 ✓ (55 select specs) · contract 13 ✓ · lint 0 err./catalog/select: basic, searchable, multi, bindLabel/bindValue, custom templates, and the "inside a modal" demo (panel stacks above the dialog, focus stays trapped). Visual smoke audited: console/network clean.