fix(directory): PrincipalPicker UI polish + Radix Dialog pointer-events bug#424
Open
larsgeorge-db wants to merge 1 commit into
Open
fix(directory): PrincipalPicker UI polish + Radix Dialog pointer-events bug#424larsgeorge-db wants to merge 1 commit into
larsgeorge-db wants to merge 1 commit into
Conversation
…ts bug Stacks on Phase 4 (#417). Post-merge polish surfaced during manual QA against the file-backed directory provider. Pointer-events (root cause): - Radix Dialog locks ``pointer-events: none`` on <body> while open. The picker's type-ahead Popover is portaled to <body> and therefore inherited the lock, making every result row unclickable inside any Dialog or Sheet host (Role form, Team form, Owner dialog, DC wizard, workflow designer Sheet, comment sidebar Sheet, etc.). Clicks landed on the dialog's own help-text <p> instead. Verified end-to-end via Playwright (elementsFromPoint returned the dialog subtree). - Fix: explicit ``style={{ pointerEvents: 'auto' }}`` on the picker's PopoverContent so the portaled popover subtree overrides the body lock. One-line change inside PrincipalPicker covers all 18 call sites without touching any consumer. Browse sub-dialog UX: - Always close after a pick, even in multi-select. Browse is the discovery path; the inline type-ahead is the fast path for adding several entries in succession. Avoids the previous confusing "added but the dialog stayed open" behaviour. - Reset query + filter when the dialog re-opens (via useEffect on ``open``), not on the close path. Several close paths (Close button, parent state change, single-select auto-close) bypassed the prior onOpenChange-based reset, so re-opening showed stale search text. Inline type-ahead focus: - onMouseDown={preventDefault} on PrincipalRow keeps focus on the input so the popover's dismiss-on-blur doesn't race the click handler. Defensive even with the pointer-events fix in place. Layout + labels: - SelectionBadges now render below the input row (was inline). Pills can grow tall on multi-select; below-input layout matches Gmail- style chip lists and keeps the input width stable. - Drop the stale "(comma-separated)" suffix from the Assigned Directory Groups label. With the directory configured the badges are discrete, not comma-separated, so the hint was misleading. Self-rendered manual-mode hint inside the picker shows "Press Enter, Tab, or comma to add an entry." only when the directory is unconfigured. i18n updated across en, de, es, fr, it, ja, nl. Tests: - principal-picker.test.tsx: 19 passed (was 13). New cases cover the badge-below-input order, dialog reset-on-open, manual-mode hint visibility, close-after-pick in multi-select, and a regression test asserting row mousedown is default-prevented. Verified live via Playwright: inline pick (Edit Role -> "pro" -> Data Producers added as badge, input cleared); Browse dialog pick (search "con" -> Data Consumers added, dialog closed); reset-on- reopen confirmed empty input after closing with text and reopening.
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.
Stacks on #417 (Phase 4). Post-merge polish surfaced during manual QA against the file-backed directory provider. Merge after #417.
Root cause: Radix Dialog locks
pointer-events: noneon<body>The picker's type-ahead
Popoveris portaled to<body>and therefore inherited that lock, making every result row unclickable inside anyDialogorSheethost (Role form, Team form, Owner dialog, DC wizard, Workflow Designer sheet, Comment sidebar sheet, etc.). Clicks landed on the dialog's own help-text<p>instead.Verified live via Playwright:
elementsFromPoint(rowCenter)returned the dialog subtree, and ancestor walk on the popover row showedpointer-events: nonepropagating all the way from<body>:Fix: explicit
style={{ pointerEvents: 'auto' }}on PrincipalPicker'sPopoverContent. One-line change in the component covers all 18 call sites without touching any consumer.Other UX issues fixed in the same pass
useEffectonopen. Several close paths (Close button, parent state change, single-select auto-close) bypassed the prioronOpenChange-based reset, so re-opening showed stale search text.onMouseDown={preventDefault}keeps focus on the input so the popover's dismiss-on-blur can't race the click handler. Defensive even with the pointer-events fix in place.SelectionBadgesrendered below the input row (was inline). Pills can grow tall on multi-select; below-input layout matches Gmail-style chip lists and keeps the input width stable.Audit: all 18 call sites benefit
Because the fix lives inside
PrincipalPicker, every consumer inherits it. Hosts surveyed:Sheet behaves identically to Dialog (both built on
@radix-ui/react-dialog), so the same fix unbroke the workflow-designer Sheet pickers and the comment sidebar Sheet picker. The page-level entitlements call is unaffected — the fix is a no-op there.Tests
principal-picker.test.tsx: 19 passed (was 13). New cases:Manual verification (Playwright-driven live browser)
pro→ popover opens with "Data Producers" rowcon→ click "Data Consumers" row → badge added, dialog closed ✓constill in input → input is empty ✓Test plan
Caveat (out of scope)
The underlying Radix pattern affects any portaled interactive content (custom Popover, DropdownMenu, Combobox/cmdk) rendered inside our Dialogs/Sheets. This PR fixes the directory lookup path. A broader codebase sweep for the same pattern is a follow-up.