From dab7cbc9da37c8017b60bb913ec8f0f14d32c2b8 Mon Sep 17 00:00:00 2001
From: Supra4E8C <tan13318527080@163.com>
Date: Wed, 30 Sep 2026 21:41:05 +0800
Subject: [PATCH] fix(providers): use shared selects in advanced editors

---
 src/components/ui/Select.tsx                  |  9 ++++
 .../sheets/forms/ErrorRulesEditor.tsx         | 52 +++++++++----------
 .../sheets/forms/ModelAdvancedFields.tsx      | 27 ++++++----
 .../sheets/forms/ProviderBehaviorEditor.tsx   | 25 +++++----
 .../sheets/forms/RuntimePolicyEditor.tsx      | 42 +++++++--------
 tests/providerErrorRulesUi.test.ts            |  5 +-
 tests/providerModelOptions.test.ts            |  7 ++-
 tests/providerSelectUi.test.ts                | 49 +++++++++++++++++
 8 files changed, 144 insertions(+), 72 deletions(-)
 create mode 100644 tests/providerSelectUi.test.ts

diff --git a/src/components/ui/Select.tsx b/src/components/ui/Select.tsx
index 08d845d3..736cf631 100644
--- a/src/components/ui/Select.tsx
+++ b/src/components/ui/Select.tsx
@@ -7,6 +7,7 @@ import {
   useRef,
   useState,
   type CSSProperties,
+  type AriaAttributes,
 } from 'react';
 import { createPortal } from 'react-dom';
 import { IconChevronDown } from './icons';
@@ -27,6 +28,7 @@ interface SelectProps {
   ariaLabel?: string;
   ariaLabelledBy?: string;
   ariaDescribedBy?: string;
+  ariaInvalid?: AriaAttributes['aria-invalid'];
   fullWidth?: boolean;
   size?: 'sm' | 'md';
   id?: string;
@@ -86,6 +88,7 @@ export function Select({
   ariaLabel,
   ariaLabelledBy,
   ariaDescribedBy,
+  ariaInvalid,
   fullWidth = true,
   size = 'md',
   id,
@@ -96,6 +99,7 @@ export function Select({
   const [open, setOpen] = useState(false);
   const [highlightedIndex, setHighlightedIndex] = useState(-1);
   const wrapRef = useRef<HTMLDivElement | null>(null);
+  const triggerRef = useRef<HTMLButtonElement | null>(null);
   const dropdownRef = useRef<HTMLDivElement | null>(null);
   const rafRef = useRef<number | null>(null);
   const [dropdownStyle, setDropdownStyle] = useState<CSSProperties | null>(null);
@@ -191,6 +195,7 @@ export function Select({
       onChange(nextOption.value);
       setOpen(false);
       setHighlightedIndex(nextIndex);
+      triggerRef.current?.focus({ preventScroll: true });
     },
     [onChange, options]
   );
@@ -250,7 +255,9 @@ export function Select({
         case 'Escape':
           if (!isOpen) return;
           event.preventDefault();
+          event.stopPropagation();
           setOpen(false);
+          triggerRef.current?.focus({ preventScroll: true });
           return;
         case 'Tab':
           if (isOpen) setOpen(false);
@@ -310,6 +317,7 @@ export function Select({
       >
         <button
           id={selectId}
+          ref={triggerRef}
           type="button"
           className={`${styles.trigger} ${size === 'sm' ? styles.triggerSm : ''}`.trim()}
           onClick={disabled ? undefined : () => setOpen((prev) => !prev)}
@@ -325,6 +333,7 @@ export function Select({
           aria-label={ariaLabel}
           aria-labelledby={ariaLabelledBy}
           aria-describedby={ariaDescribedBy}
+          aria-invalid={ariaInvalid}
           disabled={disabled}
         >
           <span className={`${styles.triggerText} ${isPlaceholder ? styles.placeholder : ''}`}>
diff --git a/src/features/providers/sheets/forms/ErrorRulesEditor.tsx b/src/features/providers/sheets/forms/ErrorRulesEditor.tsx
index 3742d900..7abcef46 100644
--- a/src/features/providers/sheets/forms/ErrorRulesEditor.tsx
+++ b/src/features/providers/sheets/forms/ErrorRulesEditor.tsx
@@ -1,6 +1,7 @@
 import { useEffect, useId, useRef, useState } from 'react';
 import { useTranslation } from 'react-i18next';
 import { IconChevronDown, IconChevronUp, IconPlus, IconTrash2 } from '@/components/ui/icons';
+import { Select } from '@/components/ui/Select';
 import {
   ERROR_RULE_ACTIONS,
   createErrorMatch,
@@ -121,25 +122,25 @@ export function ErrorRulesEditor({ rules, onChange, disabled }: ErrorRulesEditor
                 <label htmlFor={`${id}-${rule.id}-action`} className={form.label}>
                   {t(`${key}.action`)}
                 </label>
-                <select
+                <Select
                   id={`${id}-${rule.id}-action`}
-                  className={form.input}
+                  ariaLabel={t(`${key}.action`)}
                   value={rule.action}
                   disabled={disabled}
-                  aria-invalid={actionError || undefined}
-                  aria-describedby={actionError ? errorId : undefined}
-                  onChange={(event) => update({ ...rule, action: event.target.value })}
-                >
-                  <option value="">{t(`${key}.selectAction`)}</option>
-                  {rule.action && !actions.includes(rule.action) && (
-                    <option value={rule.action}>{rule.action}</option>
-                  )}
-                  {actions.map((action) => (
-                    <option key={action} value={action}>
-                      {t(`${key}.actions.${action}`)}
-                    </option>
-                  ))}
-                </select>
+                  ariaInvalid={actionError || undefined}
+                  ariaDescribedBy={actionError ? errorId : undefined}
+                  options={[
+                    { value: '', label: t(`${key}.selectAction`) },
+                    ...(rule.action && !actions.includes(rule.action)
+                      ? [{ value: rule.action, label: rule.action }]
+                      : []),
+                    ...actions.map((action) => ({
+                      value: action,
+                      label: t(`${key}.actions.${action}`),
+                    })),
+                  ]}
+                  onChange={(action) => update({ ...rule, action })}
+                />
               </div>
             </div>
             {rule.matches.map((match, matchIndex) => (
@@ -148,25 +149,24 @@ export function ErrorRulesEditor({ rules, onChange, disabled }: ErrorRulesEditor
                   <label htmlFor={`${id}-${match.id}-kind`} className={form.label}>
                     {t(`${key}.matchType`)}
                   </label>
-                  <select
+                  <Select
                     id={`${id}-${match.id}-kind`}
-                    className={form.input}
+                    ariaLabel={t(`${key}.matchType`)}
                     value={match.kind}
                     disabled={disabled}
-                    onChange={(event) =>
+                    options={[
+                      { value: 'text', label: t(`${key}.text`) },
+                      { value: 'regex', label: t(`${key}.regex`) },
+                    ]}
+                    onChange={(kind) =>
                       update({
                         ...rule,
                         matches: rule.matches.map((item) =>
-                          item.id === match.id
-                            ? { ...item, kind: event.target.value as 'text' | 'regex' }
-                            : item
+                          item.id === match.id ? { ...item, kind: kind as 'text' | 'regex' } : item
                         ),
                       })
                     }
-                  >
-                    <option value="text">{t(`${key}.text`)}</option>
-                    <option value="regex">{t(`${key}.regex`)}</option>
-                  </select>
+                  />
                 </div>
                 <div className={form.field}>
                   <label htmlFor={matchId(match.id)} className={form.label}>
diff --git a/src/features/providers/sheets/forms/ModelAdvancedFields.tsx b/src/features/providers/sheets/forms/ModelAdvancedFields.tsx
index f88c328e..94b1475f 100644
--- a/src/features/providers/sheets/forms/ModelAdvancedFields.tsx
+++ b/src/features/providers/sheets/forms/ModelAdvancedFields.tsx
@@ -1,4 +1,6 @@
+import { useId } from 'react';
 import { useTranslation } from 'react-i18next';
+import { Select } from '@/components/ui/Select';
 import { SelectionCheckbox } from '@/components/ui/SelectionCheckbox';
 import { getProviderModelCapabilities } from '../../descriptors';
 import { THINKING_LEVELS } from '../../thinkingLevels';
@@ -21,6 +23,7 @@ export function ModelAdvancedFields({
   onUpdate,
 }: ModelAdvancedFieldsProps) {
   const { t } = useTranslation();
+  const id = useId();
   const capabilities = getProviderModelCapabilities(providerBrand);
   const enabled = entry.thinkingEnabled ?? Boolean(entry.thinkingJson?.trim());
   const levels = entry.thinkingLevels ?? [];
@@ -102,18 +105,22 @@ export function ModelAdvancedFields({
       {supportsThinking ? (
         <fieldset className={styles.thinkingFieldset}>
           <legend className={styles.label}>{t('providersPage.form.thinkingConfig')}</legend>
-          <label className={styles.field}>
-            <span className={styles.label}>{t('providersPage.modelOptions.thinkingMode')}</span>
-            <select
-              className={styles.input}
+          <div className={styles.field}>
+            <label htmlFor={`${id}-thinking-mode`} className={styles.label}>
+              {t('providersPage.modelOptions.thinkingMode')}
+            </label>
+            <Select
+              id={`${id}-thinking-mode`}
+              ariaLabel={t('providersPage.modelOptions.thinkingMode')}
               value={enabled ? 'custom' : 'default'}
               disabled={disabled}
-              onChange={(event) => onUpdate({ thinkingEnabled: event.target.value === 'custom' })}
-            >
-              <option value="default">{t('providersPage.modelOptions.useDefault')}</option>
-              <option value="custom">{t('providersPage.modelOptions.custom')}</option>
-            </select>
-          </label>
+              options={[
+                { value: 'default', label: t('providersPage.modelOptions.useDefault') },
+                { value: 'custom', label: t('providersPage.modelOptions.custom') },
+              ]}
+              onChange={(mode) => onUpdate({ thinkingEnabled: mode === 'custom' })}
+            />
+          </div>
           <p className={styles.thinkingExistingHint}>
             {t('providersPage.modelOptions.thinkingHint')}
           </p>
diff --git a/src/features/providers/sheets/forms/ProviderBehaviorEditor.tsx b/src/features/providers/sheets/forms/ProviderBehaviorEditor.tsx
index 3657504f..27a14eb6 100644
--- a/src/features/providers/sheets/forms/ProviderBehaviorEditor.tsx
+++ b/src/features/providers/sheets/forms/ProviderBehaviorEditor.tsx
@@ -1,6 +1,7 @@
 import { useId } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Collapsible } from '@/components/ui/Collapsible';
+import { Select } from '@/components/ui/Select';
 import type { ProviderBehaviorOptions } from '@/types/provider';
 import { getProviderBehaviorCapabilities } from '../../descriptors';
 import type { ProviderBrand } from '../../types';
@@ -48,9 +49,9 @@ export function ProviderBehaviorEditor({
             <label htmlFor={`${id}-cloak`} className={styles.label}>
               {t('providersPage.behavior.codexCloaking')}
             </label>
-            <select
+            <Select
               id={`${id}-cloak`}
-              className={styles.input}
+              ariaLabel={t('providersPage.behavior.codexCloaking')}
               value={
                 value.disableCodexCloaking === undefined
                   ? 'default'
@@ -59,20 +60,18 @@ export function ProviderBehaviorEditor({
                     : 'enabled'
               }
               disabled={disabled}
-              aria-describedby={`${id}-cloak-hint`}
-              onChange={(event) =>
+              ariaDescribedBy={`${id}-cloak-hint`}
+              options={[
+                { value: 'default', label: t('providersPage.behavior.useDefault') },
+                { value: 'enabled', label: t('providersPage.behavior.cloakingEnabled') },
+                { value: 'disabled', label: t('providersPage.behavior.cloakingDisabled') },
+              ]}
+              onChange={(mode) =>
                 onChange({
-                  disableCodexCloaking:
-                    event.target.value === 'default'
-                      ? undefined
-                      : event.target.value === 'disabled',
+                  disableCodexCloaking: mode === 'default' ? undefined : mode === 'disabled',
                 })
               }
-            >
-              <option value="default">{t('providersPage.behavior.useDefault')}</option>
-              <option value="enabled">{t('providersPage.behavior.cloakingEnabled')}</option>
-              <option value="disabled">{t('providersPage.behavior.cloakingDisabled')}</option>
-            </select>
+            />
             <small id={`${id}-cloak-hint`} className={styles.labelHint}>
               {t('providersPage.behavior.codexCloakingHint')}
             </small>
diff --git a/src/features/providers/sheets/forms/RuntimePolicyEditor.tsx b/src/features/providers/sheets/forms/RuntimePolicyEditor.tsx
index 33727876..5f0b6177 100644
--- a/src/features/providers/sheets/forms/RuntimePolicyEditor.tsx
+++ b/src/features/providers/sheets/forms/RuntimePolicyEditor.tsx
@@ -1,6 +1,7 @@
 import { useId } from 'react';
 import { useTranslation } from 'react-i18next';
 import { Collapsible } from '@/components/ui/Collapsible';
+import { Select } from '@/components/ui/Select';
 import type { RuntimePolicyDraft } from '../../runtimePolicy';
 import { ErrorRulesEditor } from './ErrorRulesEditor';
 import styles from './sharedForm.module.scss';
@@ -32,19 +33,20 @@ export function RuntimePolicyEditor({
           <label htmlFor={`${id}-cooling`} className={styles.label}>
             {t(`${key}.cooling`)}
           </label>
-          <select
+          <Select
             id={`${id}-cooling`}
-            className={styles.input}
+            ariaLabel={t(`${key}.cooling`)}
             value={value.cooling}
             disabled={disabled}
-            onChange={(event) =>
-              onChange({ ...value, cooling: event.target.value as RuntimePolicyDraft['cooling'] })
+            options={[
+              { value: 'inherit', label: t(`${key}.inherit`) },
+              { value: 'enabled', label: t(`${key}.coolingEnabled`) },
+              { value: 'disabled', label: t(`${key}.coolingDisabled`) },
+            ]}
+            onChange={(cooling) =>
+              onChange({ ...value, cooling: cooling as RuntimePolicyDraft['cooling'] })
             }
-          >
-            <option value="inherit">{t(`${key}.inherit`)}</option>
-            <option value="enabled">{t(`${key}.coolingEnabled`)}</option>
-            <option value="disabled">{t(`${key}.coolingDisabled`)}</option>
-          </select>
+          />
         </div>
         <div className={styles.field}>
           <label htmlFor={`${id}-retry`} className={styles.label}>
@@ -70,22 +72,20 @@ export function RuntimePolicyEditor({
               <label htmlFor={`${id}-errors-mode`} className={styles.label}>
                 {t(`${key}.errorsMode`)}
               </label>
-              <select
+              <Select
                 id={`${id}-errors-mode`}
-                className={styles.input}
+                ariaLabel={t(`${key}.errorsMode`)}
                 value={value.errorsMode}
                 disabled={disabled}
-                aria-describedby={`${id}-errors-hint`}
-                onChange={(event) =>
-                  onChange({
-                    ...value,
-                    errorsMode: event.target.value as RuntimePolicyDraft['errorsMode'],
-                  })
+                ariaDescribedBy={`${id}-errors-hint`}
+                options={[
+                  { value: 'inherit', label: t(`${key}.inherit`) },
+                  { value: 'override', label: t(`${key}.override`) },
+                ]}
+                onChange={(errorsMode) =>
+                  onChange({ ...value, errorsMode: errorsMode as RuntimePolicyDraft['errorsMode'] })
                 }
-              >
-                <option value="inherit">{t(`${key}.inherit`)}</option>
-                <option value="override">{t(`${key}.override`)}</option>
-              </select>
+              />
               <span id={`${id}-errors-hint`} className={styles.labelHint}>
                 {t(`${key}.errorsHint`)}
               </span>
diff --git a/tests/providerErrorRulesUi.test.ts b/tests/providerErrorRulesUi.test.ts
index 46463f2f..b52bbcce 100644
--- a/tests/providerErrorRulesUi.test.ts
+++ b/tests/providerErrorRulesUi.test.ts
@@ -37,6 +37,8 @@ describe('error-rule editor UI', () => {
     expect(html).toContain('Add rule');
     expect(html).toContain('Add condition');
     expect(html).not.toContain('JSON');
+    expect(html).not.toContain('<select');
+    expect(html).toContain('aria-haspopup="listbox"');
     const ids = [...html.matchAll(/\bid="([^"]+)"/g)].map((match) => match[1]);
     expect(new Set(ids).size).toBe(ids.length);
     for (const label of html.matchAll(/\bfor="([^"]+)"/g)) expect(ids).toContain(label[1]);
@@ -90,7 +92,8 @@ describe('error-rule editor UI', () => {
     const html = renderRows([{ ...rows[0], action: '<unknown>' }]);
     expect(html).toContain('&lt;unknown&gt;');
     expect(html).not.toContain('<unknown>');
-    expect(html).toContain('value="&lt;unknown&gt;" selected=""');
+    expect(html).toContain('>&lt;unknown&gt;</span>');
+    expect(html).toContain('aria-label="Action"');
   });
   test('every locale contains complete row labels, errors, actions and no stale JSON guidance', () => {
     for (const messages of [en, zhCN, zhTW, ru]) {
diff --git a/tests/providerModelOptions.test.ts b/tests/providerModelOptions.test.ts
index be6a28e7..d6093228 100644
--- a/tests/providerModelOptions.test.ts
+++ b/tests/providerModelOptions.test.ts
@@ -213,6 +213,11 @@ describe('provider model options', () => {
     const defaultThinking = render('gemini', false, false).split('<fieldset')[1];
     const inputs = defaultThinking.match(/<(?:input|button)\b[^>]*>/g) ?? [];
     expect(inputs.length).toBeGreaterThan(4);
-    for (const input of inputs) expect(input).toContain('disabled=""');
+    const mode = inputs.filter((input) => input.includes('aria-haspopup="listbox"'));
+    expect(mode).toHaveLength(1);
+    expect(mode[0]).not.toContain('disabled=""');
+    for (const input of inputs.filter((input) => !input.includes('aria-haspopup="listbox"'))) {
+      expect(input).toContain('disabled=""');
+    }
   });
 });
diff --git a/tests/providerSelectUi.test.ts b/tests/providerSelectUi.test.ts
new file mode 100644
index 00000000..7b72052c
--- /dev/null
+++ b/tests/providerSelectUi.test.ts
@@ -0,0 +1,49 @@
+import { expect, test } from 'bun:test';
+import { readFileSync } from 'node:fs';
+import { createElement } from 'react';
+import { renderToStaticMarkup } from 'react-dom/server';
+import { Select } from '@/components/ui/Select';
+
+test('Provider advanced editors consistently use the shared Select instead of native menus', () => {
+  for (const file of [
+    'RuntimePolicyEditor',
+    'ProviderBehaviorEditor',
+    'ModelAdvancedFields',
+    'ErrorRulesEditor',
+  ]) {
+    const source = readFileSync(
+      new URL(`../src/features/providers/sheets/forms/${file}.tsx`, import.meta.url),
+      'utf8'
+    );
+    expect(source).toContain("import { Select } from '@/components/ui/Select'");
+    expect(source).toContain('<Select');
+    expect(source).not.toMatch(/<select[\s>]/);
+    expect(source).not.toMatch(/<option[\s>]/);
+  }
+});
+
+test('shared Select forwards validation, labels and disabled state to its trigger', () => {
+  const markup = renderToStaticMarkup(
+    createElement(Select, {
+      id: 'rule-action',
+      value: 'stop',
+      options: [{ value: 'stop', label: 'Stop' }],
+      onChange: () => {},
+      ariaLabel: 'Action',
+      ariaLabelledBy: 'action-label',
+      ariaDescribedBy: 'action-error',
+      ariaInvalid: true,
+      disabled: true,
+    })
+  );
+  expect(markup).not.toContain('<select');
+  expect(markup).toContain('id="rule-action"');
+  expect(markup).toContain('aria-haspopup="listbox"');
+  expect(markup).toContain('aria-expanded="false"');
+  expect(markup).toContain('aria-label="Action"');
+  expect(markup).toContain('aria-labelledby="action-label"');
+  expect(markup).toContain('aria-describedby="action-error"');
+  expect(markup).toContain('aria-invalid="true"');
+  expect(markup).toContain('disabled=""');
+  expect(markup).toContain('>Stop</span>');
+});