fix(directory): PrincipalPicker UI polish + Radix Dialog pointer-events bug

src/frontend/src/components/common/principal-picker.test.tsx

@@ -254,3 +254,128 @@ describe('PrincipalPicker — configured mode', () => {
     expect(onChange).toHaveBeenLastCalledWith(['alice@x.com']);
   });
 });
+
+describe('PrincipalPicker — manual-entry hint', () => {
+  it('renders the manual-mode hint only when directory is unconfigured', () => {
+    setDirectoryStatus(false);
+    const { rerender } = renderWithProviders(
+      <PrincipalPicker multiple value={[]} onChange={() => {}} />,
+    );
+    expect(screen.getByTestId('principal-picker-manual-hint')).toBeInTheDocument();
+
+    setDirectoryStatus(true);
+    rerender(<PrincipalPicker multiple value={[]} onChange={() => {}} />);
+    expect(screen.queryByTestId('principal-picker-manual-hint')).toBeNull();
+  });
+});
+
+describe('PrincipalPicker — pill placement', () => {
+  it('renders badges below the input row in the DOM', () => {
+    setDirectoryStatus(false);
+    renderWithProviders(
+      <PrincipalPicker multiple value={['alice@x.com']} onChange={() => {}} />,
+    );
+    const input = screen.getByTestId('principal-picker-input');
+    const badge = screen.getByTestId('principal-badge');
+    // ``compareDocumentPosition`` returns bitmask DOCUMENT_POSITION_FOLLOWING (4)
+    // when the second node follows the first. The badge should follow the input.
+    expect(input.compareDocumentPosition(badge) & Node.DOCUMENT_POSITION_FOLLOWING).toBeTruthy();
+  });
+});
+
+describe('PrincipalPicker — Browse dialog', () => {
+  beforeEach(() => {
+    setDirectoryStatus(true);
+    fetchMock.mockImplementation((url: RequestInfo | URL) => {
+      const u = url.toString();
+      if (u.includes('/api/directory/search')) {
+        return Promise.resolve(
+          new Response(
+            JSON.stringify({
+              results: [
+                { type: 'group', id: 'Producers', display_name: 'Data Producers', sub_label: 'producers-guid' },
+              ],
+            }),
+            { status: 200, headers: { 'Content-Type': 'application/json' } },
+          ),
+        );
+      }
+      return Promise.resolve(new Response('{}', { status: 200 }));
+    });
+  });
+
+  it('clears the search query and filter when the dialog is reopened', async () => {
+    renderWithProviders(
+      <PrincipalPicker multiple accepts={['group']} value={[]} onChange={() => {}} />,
+    );
+    // Open once.
+    await userEvent.click(screen.getByRole('button', { name: /browse directory/i }));
+    const dialogInput1 = await screen.findByTestId('principal-picker-dialog-input');
+    await userEvent.type(dialogInput1, 'pro');
+    expect((dialogInput1 as HTMLInputElement).value).toBe('pro');
+
+    // Close via the in-dialog Close button (the path that previously
+    // bypassed reset because it called onOpenChange directly). Radix
+    // also renders an icon-only close button with sr-only "Close" text;
+    // pick the visible footer button (the one without the sr-only span).
+    const closeButtons = screen.getAllByRole('button', { name: /^close$/i });
+    const visibleClose = closeButtons.find((b) => !b.querySelector('.sr-only'))!;
+    await userEvent.click(visibleClose);
+    await waitFor(() => {
+      expect(screen.queryByTestId('principal-picker-dialog-input')).toBeNull();
+    });
+
+    // Reopen.
+    await userEvent.click(screen.getByRole('button', { name: /browse directory/i }));
+    const dialogInput2 = await screen.findByTestId('principal-picker-dialog-input');
+    expect((dialogInput2 as HTMLInputElement).value).toBe('');
+  });
+
+  it('picks a row on click and emits the principal id', async () => {
+    const onChange = vi.fn();
+    renderWithProviders(
+      <PrincipalPicker multiple accepts={['group']} value={[]} onChange={onChange} />,
+    );
+    await userEvent.click(screen.getByRole('button', { name: /browse directory/i }));
+    const dialogInput = await screen.findByTestId('principal-picker-dialog-input');
+    await userEvent.type(dialogInput, 'pro');
+
+    const row = await screen.findByTestId('principal-row');
+    await userEvent.click(row);
+
+    expect(onChange).toHaveBeenLastCalledWith(['Producers']);
+  });
+
+  it('closes the dialog after a pick in multi-select mode', async () => {
+    // Browse is the discovery path -- it should always close after a
+    // pick so the user is returned to the form. Adding several entries
+    // is done via the inline type-ahead.
+    renderWithProviders(
+      <PrincipalPicker multiple accepts={['group']} value={[]} onChange={() => {}} />,
+    );
+    await userEvent.click(screen.getByRole('button', { name: /browse directory/i }));
+    const dialogInput = await screen.findByTestId('principal-picker-dialog-input');
+    await userEvent.type(dialogInput, 'pro');
+
+    const row = await screen.findByTestId('principal-row');
+    await userEvent.click(row);
+
+    await waitFor(() => {
+      expect(screen.queryByTestId('principal-picker-dialog-input')).toBeNull();
+    });
+  });
+
+  it('row mousedown is default-prevented to keep focus on the input (regression for fix #1)', async () => {
+    renderWithProviders(
+      <PrincipalPicker multiple accepts={['group']} value={[]} onChange={() => {}} />,
+    );
+    await userEvent.click(screen.getByRole('button', { name: /browse directory/i }));
+    const dialogInput = await screen.findByTestId('principal-picker-dialog-input');
+    await userEvent.type(dialogInput, 'pro');
+
+    const row = await screen.findByTestId('principal-row');
+    const event = new MouseEvent('mousedown', { bubbles: true, cancelable: true });
+    row.dispatchEvent(event);
+    expect(event.defaultPrevented).toBe(true);
+  });
+});

src/frontend/src/components/common/principal-picker.tsx

@@ -203,12 +203,6 @@ export function PrincipalPicker(props: PrincipalPickerProps) {
 
   return (
     <div className={cn('flex flex-col gap-1.5', className)}>
-      <SelectionBadges
-        ids={selectedIds}
-        resolved={resolved}
-        disabled={disabled}
-        onRemove={removeId}
-      />
       <div className="flex gap-1">
         {configured ? (
           <ConfiguredInput
@@ -244,6 +238,19 @@ export function PrincipalPicker(props: PrincipalPickerProps) {
           </Button>
         )}
       </div>
+      {!configured && (
+        // Self-rendered hint so individual callers don't have to track
+        // configured vs. unconfigured state for their own labels.
+        <p className="text-xs text-muted-foreground" data-testid="principal-picker-manual-hint">
+          Press Enter, Tab, or comma to add an entry.
+        </p>
+      )}
+      <SelectionBadges
+        ids={selectedIds}
+        resolved={resolved}
+        disabled={disabled}
+        onRemove={removeId}
+      />
       {configured && (
         <PrincipalPickerDialog
           open={dialogOpen}
@@ -252,7 +259,10 @@ export function PrincipalPicker(props: PrincipalPickerProps) {
           selectedIds={selectedIds}
           onPick={(p) => {
             addPrincipal(p);
-            if (!multiple) setDialogOpen(false);
+            // Always close after a pick -- even in multi-select mode.
+            // Browse is the discovery path; the inline type-ahead is the
+            // fast path for adding several entries in succession.
+            setDialogOpen(false);
           }}
           onSearchFail={markDegraded}
         />
@@ -395,6 +405,12 @@ function ConfiguredInput({
         className="w-[var(--radix-popover-trigger-width)] p-1"
         align="start"
         onOpenAutoFocus={(e) => e.preventDefault()}
+        // Radix Dialog locks ``pointer-events: none`` on <body> while open.
+        // The Popover is portaled to <body>, so it inherits that lock and
+        // every row inside becomes unclickable when this picker is used
+        // inside a dialog (e.g. Role form, Team form, Owner dialog).
+        // Restoring auto here re-enables clicks for the popover subtree.
+        style={{ pointerEvents: 'auto' }}
       >
         {loading ? (
           <div className="flex items-center gap-2 px-2 py-2 text-sm text-muted-foreground">
@@ -481,6 +497,10 @@ function PrincipalRow({ principal, onPick }: PrincipalRowProps) {
       <button
         type="button"
         onClick={onPick}
+        // Keep focus on the anchoring input so the surrounding popover /
+        // dialog focus management does not unmount the row before React
+        // dispatches the click handler.
+        onMouseDown={(e) => e.preventDefault()}
         className="w-full flex items-center gap-2 px-2 py-1.5 rounded-sm text-left hover:bg-accent focus:bg-accent focus:outline-none"
         data-testid="principal-row"
       >
@@ -525,14 +545,23 @@ function PrincipalPickerDialog({
     onError: onSearchFail,
   });
 
-  // Reset query and type filter whenever the dialog transitions to
-  // closed. Done in the onOpenChange handler (not an effect) to keep
-  // state writes out of effect bodies.
-  const handleOpenChange = (next: boolean) => {
-    if (!next) {
+  // Reset on the open transition rather than close, because the dialog
+  // may be closed via paths that bypass ``handleOpenChange`` -- the
+  // Close button below calls ``onOpenChange`` directly, and the
+  // single-select parent closes the dialog from outside. Resetting on
+  // open is the only way to guarantee a clean slate every time.
+  useEffect(() => {
+    if (open) {
       setQuery('');
       setFilter(accepts);
     }
+    // ``accepts`` intentionally omitted -- callers pass a fresh array
+    // every render, which would re-run this effect and wipe state on
+    // every parent re-render.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [open]);
+
+  const handleOpenChange = (next: boolean) => {
     onOpenChange(next);
   };
 

src/frontend/src/i18n/locales/de/settings.json

@@ -200,7 +200,7 @@
       "roleName": "Rollenname",
       "roleNameRequired": "Rollenname ist erforderlich",
       "description": "Beschreibung",
-      "assignedGroups": "Zugewiesene Verzeichnisgruppen (durch Komma getrennt)",
+      "assignedGroups": "Zugewiesene Verzeichnisgruppen",
       "assignedGroupsPlaceholder": "z.B. data-stewards, finance-team",
       "assignedGroupsHelp": "Benutzer dieser Gruppen erben die Berechtigungen dieser Rolle."
     },

src/frontend/src/i18n/locales/en/settings.json

@@ -293,7 +293,7 @@
       "roleName": "Role Name",
       "roleNameRequired": "Role name is required",
       "description": "Description",
-      "assignedGroups": "Assigned Directory Groups (comma-separated)",
+      "assignedGroups": "Assigned Directory Groups",
       "assignedGroupsPlaceholder": "e.g., data-stewards, finance-team",
       "assignedGroupsHelp": "Users belonging to these groups will inherit this role's permissions."
     },

src/frontend/src/i18n/locales/es/settings.json

@@ -200,7 +200,7 @@
       "roleName": "Nombre del rol",
       "roleNameRequired": "El nombre del rol es obligatorio",
       "description": "Descripción",
-      "assignedGroups": "Grupos de directorio asignados (separados por comas)",
+      "assignedGroups": "Grupos de directorio asignados",
       "assignedGroupsPlaceholder": "ej. data-stewards, finance-team",
       "assignedGroupsHelp": "Los usuarios pertenecientes a estos grupos heredarán los permisos de este rol."
     },

src/frontend/src/i18n/locales/fr/settings.json

@@ -200,7 +200,7 @@
       "roleName": "Nom du rôle",
       "roleNameRequired": "Le nom du rôle est requis",
       "description": "Description",
-      "assignedGroups": "Groupes d'annuaire assignés (séparés par des virgules)",
+      "assignedGroups": "Groupes d'annuaire assignés",
       "assignedGroupsPlaceholder": "ex. data-stewards, finance-team",
       "assignedGroupsHelp": "Les utilisateurs appartenant à ces groupes hériteront des autorisations de ce rôle."
     },

src/frontend/src/i18n/locales/it/settings.json

@@ -200,7 +200,7 @@
       "roleName": "Nome ruolo",
       "roleNameRequired": "Il nome del ruolo è obbligatorio",
       "description": "Descrizione",
-      "assignedGroups": "Gruppi di directory assegnati (separati da virgola)",
+      "assignedGroups": "Gruppi di directory assegnati",
       "assignedGroupsPlaceholder": "es. data-stewards, finance-team",
       "assignedGroupsHelp": "Gli utenti appartenenti a questi gruppi erediteranno i permessi di questo ruolo."
     },

src/frontend/src/i18n/locales/ja/settings.json

@@ -200,7 +200,7 @@
       "roleName": "ロール名",
       "roleNameRequired": "ロール名は必須です",
       "description": "説明",
-      "assignedGroups": "割り当てられたディレクトリグループ（カンマ区切り）",
+      "assignedGroups": "割り当てられたディレクトリグループ",
       "assignedGroupsPlaceholder": "例：data-stewards, finance-team",
       "assignedGroupsHelp": "これらのグループに所属するユーザーは、このロールの権限を継承します。"
     },

src/frontend/src/i18n/locales/nl/settings.json

@@ -200,7 +200,7 @@
       "roleName": "Rolnaam",
       "roleNameRequired": "Rolnaam is verplicht",
       "description": "Beschrijving",
-      "assignedGroups": "Toegewezen Directory Groepen (komma-gescheiden)",
+      "assignedGroups": "Toegewezen Directory Groepen",
       "assignedGroupsPlaceholder": "bijv. data-stewards, finance-team",
       "assignedGroupsHelp": "Gebruikers die tot deze groepen behoren zullen de machtigingen van deze rol erven."
     },