fix(ui): improve composer focus, keyboard navigation, and settings (#3376)
* fix(ui): make composer keyboard interactions consistent * docs(settings): refine description visibility guidance --------- Co-authored-by: Bohdan Triapitsyn <artmore@protonmail.com>
This commit is contained in:
committed by
GitHub
co-authored by
Bohdan Triapitsyn
parent
1306b1124c
commit
5ae1a949c8
@@ -23,7 +23,7 @@ divs — use the primitives, and extend them (in the shared file) when a new
|
||||
shape is genuinely missing.
|
||||
|
||||
- Flat hierarchy through spacing and typography; no cards, boxed backgrounds, or row chrome.
|
||||
- Secondary helper text is hidden behind an info icon (`info` prop); the default view stays quiet.
|
||||
- Secondary helper text is hidden behind an info icon (`info` prop) by default; the default view stays quiet.
|
||||
- Controls have one standard size (`h-9` / select `size="settings"`) and capped widths — no full-bleed inputs.
|
||||
- Layouts respond to the settings pane width via container queries (`@xl:` / `@3xl:`), never viewport `sm:`/`lg:` breakpoints (the pane is much narrower than the viewport inside the dialog).
|
||||
- Checkbox/radio state comes before labels; selected states are subtle and never shift layout.
|
||||
@@ -57,7 +57,8 @@ Do not introduce raw `<Tooltip>`-based info icons, direct Remixicon components,
|
||||
|
||||
## Description Policy (info hints)
|
||||
|
||||
- Explanatory prose (what a feature does, when it applies) goes behind the info icon via the `info` prop — never as always-visible `description`.
|
||||
- Explanatory prose goes behind the info icon via the `info` prop by default.
|
||||
- When labels alone cannot explain the differences, consequences, or conditions needed to choose a setting, use a title, a visible description, then checkbox or radio controls. Large-text paste modes and send shortcuts with expanded-composer exceptions need this explanation. Having multiple options or a group title alone does not require a description; see `references/controls.md` for composition.
|
||||
- Stays visible: security/data-loss warnings, destructive consequences, required syntax/placeholder lists the user reads while typing, dynamic status, empty states, validation errors, active-flow wizard instructions.
|
||||
- Mixed text: keep the warning sentence visible, move the explanation to `info`.
|
||||
|
||||
@@ -80,7 +81,7 @@ Dynamic entity rows normally are not indexed. Load `references/search.md` for ex
|
||||
## Completion Criteria
|
||||
|
||||
- Built from shared primitives; no ad-hoc page/section/row markup.
|
||||
- Explanatory text hidden behind `info`; warnings/syntax/status still visible.
|
||||
- Description placement follows the policy above; warnings/syntax/status remain visible.
|
||||
- Container-query (`@xl:`/`@3xl:`) responsiveness — no viewport breakpoints in pane content.
|
||||
- Controls use the standard size and width caps; no stretched full-width inputs.
|
||||
- Localized visible and accessibility text everywhere.
|
||||
|
||||
@@ -38,6 +38,9 @@ cells or when the control is wide; same `info` / `settingsItem` props.
|
||||
|
||||
## Boolean
|
||||
|
||||
For a self-explanatory enable/disable setting, use only a checkbox and label;
|
||||
no separate group title or description is needed.
|
||||
|
||||
```tsx
|
||||
<SettingsCheckboxRow
|
||||
checked={value}
|
||||
@@ -54,10 +57,21 @@ for text that must stay visible (warnings, dynamic status).
|
||||
|
||||
## Mutually Exclusive Options
|
||||
|
||||
Use radios for mutually exclusive modes, not independent checkboxes. Keep
|
||||
self-explanatory choices compact; a title or description is not mandatory.
|
||||
|
||||
When the skill's Description Policy calls for a visible explanation, wrap
|
||||
the controls in `SettingsControlGroup`: title, description, then options.
|
||||
Explain the choice once at group level. Use checkbox rows for independent
|
||||
choices and radio options for mutually exclusive choices. Group spacing is
|
||||
defined in `layout.md`.
|
||||
|
||||
```tsx
|
||||
<SettingsRadioGroup aria-label={t('...group')}>
|
||||
<SettingsRadioOption selected={…} onSelect={…} label={t('...')} ariaLabel={t('...')} />
|
||||
</SettingsRadioGroup>
|
||||
<SettingsControlGroup title={t('...group')} description={t('...description')}>
|
||||
<SettingsRadioGroup aria-label={t('...group')}>
|
||||
<SettingsRadioOption selected={…} onSelect={…} label={t('...')} ariaLabel={t('...')} />
|
||||
</SettingsRadioGroup>
|
||||
</SettingsControlGroup>
|
||||
```
|
||||
|
||||
Skip per-option descriptions when labels are self-explanatory. For short
|
||||
|
||||
@@ -29,7 +29,7 @@ All primitives and class constants below live in
|
||||
| L2 | `SettingsSection` title (`SETTINGS_SECTION_TITLE_CLASS`) | Section |
|
||||
| L3 | `SettingsControlGroup` title (`SETTINGS_GROUP_TITLE_CLASS`) | Sub-cluster inside a section |
|
||||
| L4 | `SETTINGS_FIELD_LABEL_CLASS` | Field / control labels |
|
||||
| Helper | `SETTINGS_HELPER_CLASS`, `SETTINGS_DESCRIPTION_CLASS` | Rare visible helper text (most goes behind `info`) |
|
||||
| Helper | `SETTINGS_HELPER_CLASS`, `SETTINGS_DESCRIPTION_CLASS` | Rare visible helper text (most goes behind `info`; see the skill's Description Policy) |
|
||||
|
||||
## Navigation Placement
|
||||
|
||||
@@ -63,5 +63,6 @@ pattern when touching nav.
|
||||
- Sections own vertical rhythm: divider + `py-8` come from `SettingsSection`.
|
||||
- Fields inside a column: `SETTINGS_FIELDS_STACK_CLASS` (`space-y-4`).
|
||||
- Checkbox/radio lists: `SETTINGS_OPTION_STACK_CLASS` (`space-y-1.5`).
|
||||
- Groups requiring a title and visible description: separate them from preceding controls with `space-y-6` on the parent. Keep simple checkbox/radio lists compact. The title sits closer to its own description and controls than to the preceding group; use `SettingsControlGroup`'s internal spacing.
|
||||
- Two-column areas: `SettingsTwoColumn` (`@3xl:grid-cols-2`); use `SettingsStackedField` inside cells (a `SettingsFieldRow` overflows half-width columns).
|
||||
- No elevated backgrounds, rounded rows, or hover fills without explicit UX value.
|
||||
|
||||
@@ -61,6 +61,14 @@ Use `Button` from `packages/ui/src/components/ui/button.tsx`.
|
||||
|
||||
Do not hardcode button height/padding when a size variant exists. Do not recreate selection/destructive styling with ad-hoc classes.
|
||||
|
||||
## Keyboard Navigation Contract
|
||||
|
||||
- Menus, selects, and autocomplete pickers with ArrowDown/ArrowUp navigation must also support Ctrl+N/Ctrl+P, including submenus and searchable lists.
|
||||
- Keep this behavior in shared components so callers inherit it. Use the keyboard mapping in `packages/ui/src/components/ui/dropdown-navigation.ts`; feature code must not duplicate key detection.
|
||||
- Lists that own their active option or stop keyboard propagation must call the shared navigation helper at their own event boundary. Wrapping a custom list in a dropdown does not guarantee that its navigation events reach the wrapper.
|
||||
- Route both key pairs through the same selection logic, preserving disabled-item skipping, boundary or wrap behavior, highlight, and scroll visibility. Consume each navigation event once, only while the menu or picker is active; preserve IME text entry and other modifier chords.
|
||||
- Verify Ctrl+N/P alongside arrow keys in the real component, including search-input focus, submenus, and closed state. A key-mapping unit test alone does not verify event propagation or focus behavior.
|
||||
|
||||
## Icon Contract
|
||||
|
||||
```tsx
|
||||
@@ -82,6 +90,7 @@ For any other technique, load `performance-engineering` and `scripts/perf/DOCUME
|
||||
- Animations are limited to `transform` and `opacity`, or their cost was measured and accepted.
|
||||
- No hardcoded/palette colors were introduced.
|
||||
- Buttons use shared variants and sizes.
|
||||
- Menus and pickers satisfy the keyboard navigation contract without caller-specific key handling for standard shared components.
|
||||
- Icons use `Icon`/`IconName`, and generated sprite changes are intentional.
|
||||
- Hover, selection, primary, and status semantics are distinct.
|
||||
- Light/dark/high-contrast and long-text states remain legible.
|
||||
|
||||
Reference in New Issue
Block a user