Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/complexselector-attached-status-icon.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@astryxdesign/core': patch
---

[fix] `ComplexSelector` with a status and the default `attached` placement now shows the status icon on its trigger in place of the chevron, as `Selector` and `MultiSelector` do, so an error is no longer shown by color alone on the field.

@duytbp
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ Consumer migration instructions belong in consumer docs and release notes.
| FR4 | Built-in Field metadata MUST name and describe the combobox trigger and preserve description, status, validation, and requirement semantics; required state reaches the trigger as `aria-required`, which the combobox role supports. | Current source; `component:Field`; DEC-1 |
| FR5 | Explicit disabled MUST expose `aria-disabled` on the trigger, built-in or caller-rendered, while it keeps its tab stop, and MUST make click, ArrowDown, imperative `open`, and imperative `toggle` no-ops; a press on a disabled trigger reaches no handler. | Current source, tests, `spec:AST-085/FR1, FR2, FR4` |
| FR6 | External `isLoading` or a pending `changeAction` MUST resolve to one busy state. The built-in path renders one decorative Spinner, exposes `aria-busy`, and announces the busy start once; caller content receives `isBusy`; caller-rendered mode receives current `aria-busy`. | Current source, docs, tests, `family:input-fields/FR5–FR6` |
| FR7 | Warning, error, and success MUST delegate to Field status. Error MUST expose `aria-invalid` on the built-in trigger. Secondary and ghost with requested attached status MUST resolve to detached status. | Current source, tests, `family:input-fields/FR8` |
| FR7 | Warning, error, and success MUST delegate to Field status. Error MUST expose `aria-invalid` on the built-in trigger. With attached status the trigger row MUST show the status icon in place of the chevron; with tooltip status it MUST show, in place of the chevron, a focusable status button that holds the message and describes the trigger. Secondary and ghost with requested attached status MUST resolve to detached status. | Current source, tests, `family:input-fields/FR8` |
| FR8 | Trigger click toggles the popup; ArrowDown opens a closed popup; content and the imperative handle can close it; every open/close route reports through `onOpenChange`. With the built-in trigger, closing returns focus to the trigger only when focus would otherwise be lost (`family:overlay-dismissal` FR9); a close caused by moving focus elsewhere leaves focus there; a caller's control gets focus back on every close (`spec:AST-055` AR3). | Current source, tests, `family:overlay-dismissal` |
| FR9 | Placement and alignment stay logical. The popup anchors to the current trigger, keeps clearance on the edge facing it (carried across a flip), and keeps content reachable through the layer-runtime fallback and scroll behavior. | Current source; `architecture:layer-runtime` |
| FR10 | With `renderTrigger`, the caller control MUST render unwrapped as the only opener, replace Field and built-in chrome, and carry the exported ref, handlers, combobox role, popup props (`aria-haspopup="dialog"`, `aria-expanded`, `aria-controls`), name, busy, required, and disabled state, plus the caller's inherited BaseProps. It anchors the popup, receives focus return, and preserves the same content, handle, and open-change behavior. | `spec:AST-055/FR1–FR7, AR1–AR4`, DEC-1, current source and tests |
Expand Down Expand Up @@ -234,7 +234,7 @@ not a ComplexSelector-owned painting surface or theme target.
| FR2, FR3 | `ComplexSelector.test.tsx` "defaults to md and reflects explicit trigger sizes", "renders a ghost toolbar trigger with a start icon", "applies distinct trigger chrome to the secondary variant" | Sizes; variants; start icon | A wrong size, a missing start slot, or a variant without its chrome fails. Evidence gap: no test asserts the trigger height or its growth with text size (the FR3 gap). | `audit:ComplexSelector/anatomy` |
| FR4, AR1 | `ComplexSelector.test.tsx` trigger-semantics cases ("exposes the built-in trigger as a combobox that carries required state", "names the built-in trigger with the field label when the caller gives no name", the caller aria-label and aria-labelledby cases, "names the trigger by its label and shows the chosen value as its content", "keeps the field label as the name when the trigger shows the placeholder"); `ComplexSelector.fr10.test.tsx` "keeps a caller aria-describedby alongside the field description" | Named, described, required; caller names | A trigger not named or described by the field, a lost required state, or a caller name ignored fails. | `audit:ComplexSelector/accessibility` |
| FR5, AR6 | `ComplexSelector.focusableDisabled.test.tsx` cases; `ComplexSelector.test.tsx` "does not open via the imperative handle when disabled", "spreads the combobox role, required, and disabled state onto a caller control" | Disabled built-in and caller control | Native `disabled`, a lost Tab stop, an open or handler reached while disabled, a lost focus when disabled while focused, or a caller control without `aria-disabled` fails. | `audit:ComplexSelector/accessibility` |
| FR7, ORD3 | `ComplexSelector.test.tsx` statusVariant cases, including `forces detached when variant="%s" even if statusVariant="attached"`; `ComplexSelector.a11y.chromium.spec.ts` matrix rest frame (`aria-invalid` on the invalid trigger) | Attached, detached, tooltip | A status that leaves Field, a tooltip message not describing the trigger, an info-tip press that opens the popup, an attached status on secondary or ghost, or an invalid trigger without `aria-invalid` fails. | `audit:ComplexSelector/anatomy` |
| FR7, ORD3 | `ComplexSelector.test.tsx` statusVariant cases, including `forces detached when variant="%s" even if statusVariant="attached"`, "shows the status icon in place of the chevron with attached status, and the chevron with detached", and "puts the tooltip-placement message in a focusable info-tip that describes the trigger"; `ComplexSelector.a11y.chromium.spec.ts` matrix rest frame (`aria-invalid` on the invalid trigger) | Attached, detached, tooltip | A status that leaves Field, a tooltip message not describing the trigger, an info-tip press that opens the popup, an attached status on secondary or ghost, or an invalid trigger without `aria-invalid` fails. | `audit:ComplexSelector/anatomy` |
| FR8, AR3 | `ComplexSelector.test.tsx` onOpenChange cases, "Escape and a close from the content return focus to the trigger", "a light dismiss onto another control leaves focus there; onto nothing focusable it returns to the trigger", "a caller-rendered control gets focus back even after a light dismiss onto another control", "does not reopen from the trigger click that follows light dismiss", "exposes imperative open, close, toggle, and isOpen via handleRef"; `ComplexSelector.test.tsx` "opens against the caller control and returns focus to it on close" | Click, ArrowDown, Escape, light dismiss, content close, handle | A missing or repeated open-change report, a reopen from a dismissing click, or focus that does not return fails. | `audit:ComplexSelector/accessibility` |
| FR9, AR5 | `ComplexSelector.test.tsx` "gives the popup clearance on the edge facing the trigger, which a flip carries to the other side", "supports end-aligned popup positioning"; `ComplexSelector.a11y.chromium.spec.ts`; `apps/storybook/rtl-audit/targets.json` ComplexSelector entry | Start and end alignment; flip; RTL | A popup without clearance, on the wrong side, off its anchor, or past the inline viewport at 320 CSS px (the audit's narrow and coarse open frames) fails. | `audit:ComplexSelector/rtl-responsive` |
| FR10, AR2, ORD5 | `ComplexSelector.test.tsx` trigger render prop cases; `ComplexSelector.fr10.test.tsx` renderTrigger cases | Caller control closed and open; ArrowDown; base props | A wrapped or second opener, missing popup props, lost base props or ref, or focus not returned fails. | `audit:ComplexSelector/accessibility` |
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/ComplexSelector/ComplexSelector.doc.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ const anatomy = [
},
{
name: 'Indicator icon',
required: true,
required: false,
description:
'Trailing chevron that rotates to reflect whether the popup is open.',
'Trailing chevron shown when status presentation does not replace it; rotates to reflect whether the popup is open.',
},
{
name: 'Popup',
Expand Down
56 changes: 55 additions & 1 deletion packages/core/src/ComplexSelector/ComplexSelector.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,14 @@

import React, {useLayoutEffect} from 'react';
import {beforeEach, describe, expect, it, vi} from 'vitest';
import {act, fireEvent, render, screen, waitFor} from '@testing-library/react';
import {
act,
fireEvent,
render,
screen,
waitFor,
within,
} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import {Button} from '../Button/Button';
import * as stylex from '@stylexjs/stylex';
Expand Down Expand Up @@ -1118,6 +1125,49 @@ describe('ComplexSelector onOpenChange', () => {
},
);

it('shows the status icon in place of the chevron with attached status, and the chevron with detached', () => {
const {container, rerender} = render(
<ComplexSelector
label="Filter"
value={[]}
status={{type: 'error', message: 'Required'}}>
{() => <button type="button">Apply</button>}
</ComplexSelector>,
);
const controlBox = screen.getByRole('combobox', {name: 'Filter'})
.parentElement as HTMLElement;
expect(
controlBox.querySelector('.astryx-input-status-icon'),
).toBeInTheDocument();
// A plain icon, not a control: the combobox stays the one tab stop.
expect(within(controlBox).queryAllByRole('button')).toHaveLength(0);
// It takes the chevron's place, as in Selector.
expect(
controlBox.querySelector('.astryx-complex-selector-indicator-icon'),
).not.toBeInTheDocument();
rerender(
<ComplexSelector
label="Filter"
value={[]}
status={{type: 'error', message: 'Required'}}
statusVariant="detached">
{() => <button type="button">Apply</button>}
</ComplexSelector>,
);
const detachedBox = screen.getByRole('combobox', {name: 'Filter'})
.parentElement as HTMLElement;
expect(
detachedBox.querySelector('.astryx-input-status-icon'),
).not.toBeInTheDocument();
expect(
detachedBox.querySelector('.astryx-complex-selector-indicator-icon'),
).toBeInTheDocument();
// The detached message box shows its own icon.
expect(
container.querySelector('.astryx-field-status'),
).toBeInTheDocument();
});

it('renders no message box for statusVariant="tooltip"', () => {
const {container} = render(
<ComplexSelector
Expand Down Expand Up @@ -1155,6 +1205,10 @@ describe('ComplexSelector onOpenChange', () => {
expect(document.getElementById(id)).not.toBeNull();
}
expect(trigger).toHaveAccessibleDescription('Required');
// The info-tip takes the chevron's place, as in Selector.
expect(
document.querySelector('.astryx-complex-selector-indicator-icon'),
).not.toBeInTheDocument();

// Focusing the info-tip reveals the message without opening the popup.
await user.tab();
Expand Down
70 changes: 39 additions & 31 deletions packages/core/src/ComplexSelector/ComplexSelector.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,7 @@ registerEnglishFallback(english);
const CONTENT_BLOCK_CAP = '480px';

const styles = stylex.create({
// The tooltip-placement status info-tip sits beside the chevron and keeps
// its size when the trigger text is long.
// The status glyph or info-tip keeps its size when the trigger text is long.
statusTip: {
display: 'inline-flex',
alignItems: 'center',
Expand Down Expand Up @@ -598,13 +597,12 @@ export function ComplexSelector<Value>({
const contentId = useId();
const descriptionId = useId();
const statusMessageId = useId();
// With `tooltip` placement Field renders no message box, so the status
// lives in an info-tip button on the trigger row (family:input-fields FR8).
// Only that placement renders an on-field status affordance here.
// With `tooltip` placement, Field renders no message box. The info-tip
// holds the message and describes the trigger.
const isTooltipStatus = effectiveStatusVariant === 'tooltip';
const {statusIcon: statusTip, describedBy: statusTipDescribedBy} =
useInputStatusIcon({
status: isTooltipStatus ? status : undefined,
status,
statusVariant: effectiveStatusVariant,
size: 'sm',
});
Expand Down Expand Up @@ -927,32 +925,42 @@ export function ComplexSelector<Value>({
{...stylex.props(styles.trigger)}>
<span {...stylex.props(styles.triggerText)}>{triggerContent}</span>
</button>
{isRenderable(statusTip) && (
// The info-tip is its own control: a press on it, or on its message
// bubble (portaled, so outside this span in the DOM but inside it in
// React), shows the message without opening the popup.
<span onClick={stopPropagation} {...stylex.props(styles.statusTip)}>
{statusTip}
</span>
)}
{isBusy && <Spinner size="sm" aria-hidden="true" />}
<Icon
icon="chevronDown"
size="sm"
color="secondary"
// No wrapper: Icon's own span already provides the 16px box (`sm`)
// and the secondary icon color the wrapper used to set, so the glyph
// IS the trigger's icon element — one node carrying the box, the
// color, the rotation, and the theme target.
xstyle={[
styles.triggerIcon,
styles.triggerIconRotation,
isOpen && styles.triggerIconOpen,
]}
{...themeProps('complex-selector-indicator-icon', {
state: isOpen ? 'expanded' : 'collapsed',
})}
/>
{isRenderable(statusTip) &&
(isTooltipStatus ? (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Non-blocking (Miss): with statusVariant="tooltip" and a status that has no message, useInputStatusIcon returns the plain glyph, not the info-tip button, but this branch still wraps it to stop the press. Now that the glyph sits in the chevron's place, a press at the trigger's end opens nothing. Keying the wrapper on the info-tip rendering fixes it:

(isTooltipStatus && status?.message ? (

// The info-tip is its own control: a press on it, or on its
// message bubble (portaled, so outside this span in the DOM but
// inside it in React), shows the message without opening the
// popup.
<span onClick={stopPropagation} {...stylex.props(styles.statusTip)}>
{statusTip}
</span>
) : (
// The plain icon is part of the field: a press on it opens the
// popup, as a press on the trigger does.
<span {...stylex.props(styles.statusTip)}>{statusTip}</span>
))}
{/* A status icon or info-tip takes the chevron's place, as in
Selector. */}
{!isRenderable(statusTip) && (
<Icon
icon="chevronDown"
size="sm"
color="secondary"
// No wrapper: Icon's own span already provides the 16px box (`sm`)
// and the secondary icon color the wrapper used to set, so the glyph
// IS the trigger's icon element — one node carrying the box, the
// color, the rotation, and the theme target.
xstyle={[
styles.triggerIcon,
styles.triggerIconRotation,
isOpen && styles.triggerIconOpen,
]}
{...themeProps('complex-selector-indicator-icon', {
state: isOpen ? 'expanded' : 'collapsed',
})}
/>
)}
</div>

{popup}
Expand Down
Loading