fix(frontend): rename filter Clear to Select none, inline user errors

frontend/src/__tests__/column-filter-popover.test.ts

@@ -133,9 +133,12 @@ describe('categorical commits', () => {
     expect(onCommit).toHaveBeenLastCalledWith('plans', 'provider', null);
   });
 
-  test('Clear writes an explicit empty allow-list', () => {
+  test('Select none writes an explicit empty allow-list', () => {
     openFor('plans', 'provider');
-    document.querySelector<HTMLButtonElement>('.column-filter-clear')!.click();
+    const btn = document.querySelector<HTMLButtonElement>('.column-filter-clear')!;
+    // #727 (UX-28): it hides every row, so it is not labelled "Clear".
+    expect(btn.textContent).toBe('Select none');
+    btn.click();
     expect(onCommit).toHaveBeenLastCalledWith('plans', 'provider', { kind: 'set', values: [] });
     expect(box('azure').checked).toBe(false);
   });
@@ -168,7 +171,9 @@ describe('numeric commits', () => {
     filters['plans:count'] = { kind: 'expr', expr: '5' };
     openFor('plans', 'count');
     expect(exprInput().value).toBe('5');
-    document.querySelector<HTMLButtonElement>('.column-filter-clear')!.click();
+    const btn = document.querySelector<HTMLButtonElement>('.column-filter-clear')!;
+    expect(btn.textContent).toBe('Clear');
+    btn.click();
     expect(exprInput().value).toBe('');
     expect(onCommit).toHaveBeenLastCalledWith('plans', 'count', null);
   });

frontend/src/__tests__/form-validation-727.test.ts

@@ -0,0 +1,219 @@
+/**
+ * Issue #727.
+ * UX-33: user form validation must be field-level (message under the
+ * field, aria-invalid + aria-describedby, focus on the first problem), not
+ * toast-only.
+ * UX-28: the categorical filter popover's footer button selects nothing,
+ * so it is labelled "Select none"; numeric filters keep "Clear".
+ */
+import './setup';
+
+jest.mock('../api', () => ({
+  createUser: jest.fn(),
+  updateUser: jest.fn(),
+  getUser: jest.fn(),
+  listUsers: jest.fn(),
+  listGroups: jest.fn(),
+}));
+
+jest.mock('../users/userActions', () => ({
+  loadUsers: jest.fn().mockResolvedValue(undefined),
+}));
+
+import * as api from '../api';
+import * as userState from '../users/state';
+import { openCreateUserModal, openEditUserModal, saveUser } from '../users/userModals';
+import { clearFieldError, clearFieldErrors, showFieldError } from '../lib/field-errors';
+import { openHistoryColumnPopover, closeOpenHistoryPopover } from '../lib/history-filter-popover';
+
+const GROUPS = [
+  { id: 'w1', name: 'Writers', description: '', permissions: [{ action: 'update', resource: 'plans' }] },
+  { id: 'v1', name: 'Viewers', description: '', permissions: [{ action: 'view', resource: '*' }] },
+];
+
+function setUpUserForm(): void {
+  // Mirrors index.html's #user-modal, including the novalidate the fix adds.
+  document.body.innerHTML = `
+    <div id="user-modal" class="modal hidden">
+      <h2 id="user-modal-title"></h2>
+      <form id="user-form" novalidate>
+        <input type="hidden" id="user-id">
+        <div class="form-group"><input type="email" id="user-email" required></div>
+        <div id="password-fields" class="form-group">
+          <input type="password" id="user-password" minlength="12">
+          <small class="form-help">help</small>
+        </div>
+        <div class="form-group">
+          <select id="user-groups" multiple required></select>
+          <small class="form-help">At least one group is required.</small>
+        </div>
+        <button type="submit">Save User</button>
+      </form>
+    </div>`;
+}
+
+const $ = <T extends HTMLElement>(id: string): T => document.getElementById(id) as T;
+
+function errorFor(control: HTMLElement): string | null | undefined {
+  const id = control.dataset['fieldErrorId'];
+  return id ? document.getElementById(id)?.textContent : null;
+}
+
+async function submit(): Promise<void> {
+  const e = new Event('submit');
+  e.preventDefault = jest.fn();
+  await saveUser(e);
+}
+
+function selectGroups(...ids: string[]): void {
+  Array.from($<HTMLSelectElement>('user-groups').options).forEach(o => { o.selected = ids.includes(o.value); });
+}
+
+describe('lib/field-errors', () => {
+  beforeEach(() => {
+    document.body.innerHTML = '<div class="form-row"><label><input id="amt" aria-describedby="amt-help"></label></div><small id="amt-help">h</small>';
+  });
+
+  it('places the message after the anchor and links it without dropping existing help', () => {
+    const input = $<HTMLInputElement>('amt');
+    showFieldError(input, 'Too small.', input.closest('.form-row')!);
+    const id = input.dataset['fieldErrorId']!;
+    const note = document.getElementById(id)!;
+    expect(note.previousElementSibling?.classList.contains('form-row')).toBe(true);
+    expect(note.className).toBe('field-error');
+    expect(input.getAttribute('aria-invalid')).toBe('true');
+    expect(input.getAttribute('aria-describedby')).toBe(`amt-help ${id}`);
+  });
+
+  it('updates in place, and clears on the next edit, restoring aria-describedby', () => {
+    const input = $<HTMLInputElement>('amt');
+    showFieldError(input, 'First.');
+    showFieldError(input, 'Second.');
+    expect(document.querySelectorAll('.field-error')).toHaveLength(1);
+    expect(errorFor(input)).toBe('Second.');
+    input.dispatchEvent(new Event('input'));
+    expect(document.querySelector('.field-error')).toBeNull();
+    expect(input.hasAttribute('aria-invalid')).toBe(false);
+    expect(input.getAttribute('aria-describedby')).toBe('amt-help');
+  });
+
+  it('clearFieldError and clearFieldErrors are safe on untouched fields', () => {
+    const input = $<HTMLInputElement>('amt');
+    expect(() => clearFieldError(input)).not.toThrow();
+    showFieldError(input, 'x');
+    clearFieldErrors(document);
+    expect(document.querySelector('.field-error')).toBeNull();
+  });
+});
+
+describe('user form field-level validation (#727 UX-33)', () => {
+  beforeEach(() => {
+    setUpUserForm();
+    jest.clearAllMocks();
+    userState.setAvailableGroups(GROUPS as api.APIGroup[]);
+    userState.setCurrentEditingUser(null);
+    (api.createUser as jest.Mock).mockResolvedValue({});
+    (api.updateUser as jest.Mock).mockResolvedValue({});
+  });
+
+  it('marks every problem at once and focuses the first', async () => {
+    openCreateUserModal();
+    $<HTMLInputElement>('user-email').value = 'not-an-email';
+    $<HTMLInputElement>('user-password').value = 'short';
+    await submit();
+
+    expect(api.createUser).not.toHaveBeenCalled();
+    const email = $<HTMLInputElement>('user-email');
+    const password = $<HTMLInputElement>('user-password');
+    const groups = $<HTMLSelectElement>('user-groups');
+    expect(errorFor(email)).toBe('Enter a valid email address, like name@example.com.');
+    expect(errorFor(password)).toBe('Password must be at least 12 characters long');
+    expect(errorFor(groups)).toBe('At least one group is required. Select one or more groups for this user.');
+    for (const el of [email, password, groups]) {
+      expect(el.getAttribute('aria-invalid')).toBe('true');
+      expect(el.getAttribute('aria-describedby')).toBe(el.dataset['fieldErrorId']);
+    }
+    expect(document.activeElement).toBe(email);
+  });
+
+  it('asks for an email when it is blank (no native bubble with novalidate)', async () => {
+    openCreateUserModal();
+    selectGroups('w1');
+    await submit();
+    expect(api.createUser).not.toHaveBeenCalled();
+    expect(errorFor($('user-email'))).toBe('Enter an email address.');
+    expect(document.activeElement).toBe($('user-email'));
+  });
+
+  it('puts the contradictory-groups message on the groups field and still toasts it', async () => {
+    openCreateUserModal();
+    $<HTMLInputElement>('user-email').value = 'a@example.com';
+    selectGroups('w1', 'v1');
+    await submit();
+    expect(api.createUser).not.toHaveBeenCalled();
+    expect(errorFor($('user-groups'))).toContain('Contradictory');
+    expect(document.querySelector('.toast-error .toast-message')?.textContent).toContain('Contradictory');
+    expect(document.activeElement).toBe($('user-groups'));
+  });
+
+  it('clears a field message when the field is edited, then saves', async () => {
+    openCreateUserModal();
+    $<HTMLInputElement>('user-email').value = 'a@example.com';
+    await submit();
+    const groups = $<HTMLSelectElement>('user-groups');
+    expect(groups.getAttribute('aria-invalid')).toBe('true');
+    selectGroups('w1');
+    groups.dispatchEvent(new Event('change'));
+    expect(groups.hasAttribute('aria-invalid')).toBe(false);
+    expect(document.querySelector('.field-error')).toBeNull();
+    await submit();
+    expect(api.createUser).toHaveBeenCalledWith({ email: 'a@example.com', password: '', groups: ['w1'] });
+  });
+
+  it('does not check the hidden password field when editing', async () => {
+    (api.getUser as jest.Mock).mockResolvedValue({ id: 'u1', email: 'a@example.com', groups: ['w1'], mfa_enabled: false });
+    await openEditUserModal('u1');
+    $<HTMLInputElement>('user-password').value = 'short';
+    await submit();
+    expect(api.updateUser).toHaveBeenCalledWith('u1', { email: 'a@example.com', groups: ['w1'] });
+    expect(document.querySelector('.field-error')).toBeNull();
+  });
+
+  it('reopening the modal clears old messages', async () => {
+    openCreateUserModal();
+    await submit();
+    expect(document.querySelectorAll('.field-error').length).toBeGreaterThan(0);
+    openCreateUserModal();
+    expect(document.querySelector('.field-error')).toBeNull();
+    expect(document.querySelector('[aria-invalid="true"]')).toBeNull();
+  });
+});
+
+describe('history filter popover footer label (#727 UX-28)', () => {
+  afterEach(() => closeOpenHistoryPopover());
+
+  function open(kind: 'numeric' | 'categorical', onCommit = jest.fn()): HTMLButtonElement {
+    document.body.innerHTML = '<button id="anchor"></button>';
+    openHistoryColumnPopover({
+      scope: 'history', column: 'provider', anchor: $('anchor'), currentFilter: undefined, headerLabel: 'Provider', kind,
+      distinctValues: kind === 'categorical' ? ['aws', 'gcp'] : undefined, onCommit,
+    });
+    return document.querySelector<HTMLButtonElement>('.column-filter-clear')!;
+  }
+
+  it('labels the categorical button "Select none", which selects nothing', () => {
+    const onCommit = jest.fn();
+    const btn = open('categorical', onCommit);
+    expect(btn.textContent).toBe('Select none');
+    btn.click();
+    expect(onCommit).toHaveBeenLastCalledWith({ kind: 'set', values: [] });
+  });
+
+  it('keeps "Clear" for numeric filters, which removes the filter', () => {
+    const onCommit = jest.fn();
+    const btn = open('numeric', onCommit);
+    expect(btn.textContent).toBe('Clear');
+    btn.click();
+    expect(onCommit).toHaveBeenLastCalledWith(null);
+  });
+});

frontend/src/index.html

@@ -1081,7 +1081,7 @@ <h2 id="create-apikey-modal-title">Create API Key</h2>
         <div id="user-modal" class="modal hidden" role="dialog" aria-modal="true" aria-labelledby="user-modal-title">
             <div class="modal-content">
                 <h2 id="user-modal-title">Create User</h2>
-                <form id="user-form">
+                <form id="user-form" novalidate>
                     <input type="hidden" id="user-id">
                     <div class="form-group">
                         <label for="user-email">Email</label>

frontend/src/lib/column-filter-popover.ts

@@ -221,7 +221,10 @@ function buildPopover(config: ColumnFilterPopoverConfig): OpenPopover {
   const clearBtn = document.createElement('button');
   clearBtn.type = 'button';
   clearBtn.className = 'column-filter-clear';
-  clearBtn.textContent = 'Clear';
+  // Issue #727 (UX-28): on a categorical filter this button selects
+  // nothing (an explicit empty allow-list, which hides every row), so it
+  // must not be called "Clear". (All) is how the filter is removed.
+  clearBtn.textContent = config.kind === 'numeric' ? 'Clear' : 'Select none';
   clearBtn.addEventListener('click', () => {
     if (p.input) {
       p.input.value = '';

frontend/src/lib/field-errors.ts

@@ -0,0 +1,67 @@
+/**
+ * Field-level validation messages for modal forms (issue #727, UX-33).
+ *
+ * The user and group forms used to report a refused save only through a
+ * toast: it fades after a few seconds, sits away from the field, and gives
+ * a screen-reader user no cue on the control itself. showFieldError puts
+ * the message under the field (a `.field-error`, styled in base.css), sets
+ * aria-invalid and links the message with aria-describedby (appended, so
+ * existing help text stays announced). The next edit of the field clears it.
+ */
+
+let seq = 0;
+
+function errorIdFor(control: HTMLElement): string {
+  if (!control.dataset['fieldErrorId']) {
+    control.dataset['fieldErrorId'] = `${control.id || 'field'}-error-${++seq}`;
+  }
+  return control.dataset['fieldErrorId'];
+}
+
+function removeToken(list: string | null, token: string): string {
+  return (list ?? '').split(/\s+/).filter(t => t && t !== token).join(' ');
+}
+
+/**
+ * Show `message` under `control`. `anchor` is the element the message is
+ * placed after; it defaults to the control itself. Pass the enclosing row
+ * when the control sits in a flex row, so the message lands below the row
+ * instead of becoming a flex item beside the control.
+ */
+export function showFieldError(control: HTMLElement, message: string, anchor?: Element): void {
+  const id = errorIdFor(control);
+  let note = document.getElementById(id);
+  if (!note) {
+    note = document.createElement('p');
+    note.id = id;
+    note.className = 'field-error';
+    (anchor ?? control).insertAdjacentElement('afterend', note);
+  }
+  note.textContent = message;
+  control.setAttribute('aria-invalid', 'true');
+  const describedBy = removeToken(control.getAttribute('aria-describedby'), id);
+  control.setAttribute('aria-describedby', describedBy ? `${describedBy} ${id}` : id);
+
+  if (control.dataset['fieldErrorWired'] !== '1') {
+    control.dataset['fieldErrorWired'] = '1';
+    const clear = (): void => clearFieldError(control);
+    control.addEventListener('input', clear);
+    control.addEventListener('change', clear);
+  }
+}
+
+/** Remove the message and invalid marker from `control`, if any. */
+export function clearFieldError(control: HTMLElement): void {
+  const id = control.dataset['fieldErrorId'];
+  if (!id) return;
+  document.getElementById(id)?.remove();
+  control.removeAttribute('aria-invalid');
+  const describedBy = removeToken(control.getAttribute('aria-describedby'), id);
+  if (describedBy) control.setAttribute('aria-describedby', describedBy);
+  else control.removeAttribute('aria-describedby');
+}
+
+/** Clear every field error raised by showFieldError inside `root`. */
+export function clearFieldErrors(root: ParentNode): void {
+  root.querySelectorAll<HTMLElement>('[data-field-error-id]').forEach(clearFieldError);
+}

frontend/src/lib/history-filter-popover.ts

@@ -326,7 +326,10 @@ export function openHistoryColumnPopover<TColumnId extends string>(
   const clearBtn = document.createElement('button');
   clearBtn.type = 'button';
   clearBtn.className = 'column-filter-clear';
-  clearBtn.textContent = 'Clear';
+  // Issue #727 (UX-28): on a categorical filter this button selects
+  // nothing (an explicit empty allow-list, which hides every row), so it
+  // must not be called "Clear". (All) is how the filter is removed.
+  clearBtn.textContent = config.kind === 'numeric' ? 'Clear' : 'Select none';
   clearBtn.addEventListener('click', () => {
     if (config.kind === 'numeric') {
       if (inputRef) inputRef.value = '';

frontend/src/recommendations.ts

@@ -2469,7 +2469,8 @@ function buildPopoverContent(
   const clearBtn = document.createElement('button');
   clearBtn.type = 'button';
   clearBtn.className = 'column-filter-clear';
-  clearBtn.textContent = 'Clear';
+  // Categorical: selects nothing (hides every row), so not "Clear" (#727).
+  clearBtn.textContent = input ? 'Clear' : 'Select none';
   clearBtn.addEventListener('click', () => {
     if (input) {
       // Numeric column: Clear drops the expression entirely (no filter).

frontend/src/styles/forms.css

@@ -157,6 +157,13 @@ label {
     border-radius: 4px;
 }
 
+/* The rule above outranks the global [aria-invalid] border (base.css), so
+   repeat it here or a field flagged by showFieldError (#727) only turns
+   red while focused. */
+.form-group [aria-invalid="true"] {
+    border-color: var(--cudly-error);
+}
+
 .form-help {
     font-size: 0.8rem;
     color: var(--cudly-text-muted);