Skip to content

Commit 6607108

Browse files
fix: Accordion/Disclosure fixes from testing (#7218)
* remove UNSTABLE_ prefix * add aria-hidden to collapsed disclosure panels * add controlled example to s2 docs * update S2 Accordion down state * fix v3 styles (HCM, border-radius, hover when open) * remove display: none in v3 * lint * fix imports * fix v3 focus/down styles * ignore repeat keydown events * fix acts * switch onKeyDown to onPressStart * add style props to v3 Disclosure, DisclosurePanel, and DisclosureHeader * fix bottom border on individual disclosures * lint * fix same disclosure border top issue in v3 * use default case * release keydown event * fix storybook code output * fix v3 quiet HCM * docs: follow-up on accordion docs from testing (#7232) * docs: follow-up on accordion docs from testing * updates svgs for mobile, follow up from reviews * prevent outline on header on android * fix examples in useDisclosure on mobile * update focus visible * add focusProps to disclosure group example * use merge props --------- Co-authored-by: Yihui Liao <[email protected]>
1 parent e24ea2f commit 6607108

File tree

20 files changed

+291
-141
lines changed

20 files changed

+291
-141
lines changed

packages/@adobe/spectrum-css-temp/components/accordion/index.css

+9-5
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,14 @@ governing permissions and limitations under the License.
5050

5151
border-bottom: var(--spectrum-accordion-item-border-size) solid transparent;
5252

53-
&:first-of-type {
53+
border-top: var(--spectrum-accordion-item-border-size) solid var(--spectrum-alias-border-color);
54+
55+
&.in-accordion, &:first-of-type {
5456
border-top: var(--spectrum-accordion-item-border-size) solid transparent;
5557
}
5658

5759
&.spectrum-Accordion-item--quiet, &.spectrum-Accordion-item--quiet:first-of-type {
60+
border-radius: var(--spectrum-global-dimension-static-size-100);
5861
border-color: transparent;
5962
}
6063
}
@@ -97,17 +100,18 @@ governing permissions and limitations under the License.
97100
width: 100%;
98101
border-style: solid;
99102
border-color: transparent;
100-
border-radius: var(--spectrum-global-dimension-static-size-100);
101103

102104
&.focus-ring {
105+
border-radius: var(--spectrum-global-dimension-static-size-100);
103106
outline: solid 2px var(--spectrum-accordion-item-focus-ring-color);
104107
outline-offset: -4px;
105108
}
106109
}
107110

108-
.spectrum-Accordion-itemContent {
109-
padding: 0 var(--spectrum-accordion-item-content-padding) var(--spectrum-accordion-item-content-padding) var(--spectrum-accordion-item-content-padding);
110-
display: none;
111+
.spectrum-Accordion-item.is-expanded {
112+
.spectrum-Accordion-itemContent {
113+
padding: 0 var(--spectrum-accordion-item-content-padding) var(--spectrum-accordion-item-content-padding) var(--spectrum-accordion-item-content-padding);
114+
}
111115
}
112116

113117
.spectrum-Accordion-item {

packages/@adobe/spectrum-css-temp/components/accordion/skin.css

+11-11
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ governing permissions and limitations under the License.
1212

1313
:root {
1414
--spectrum-accordion-text-color-disabled: var(--spectrum-alias-text-color-disabled);
15+
--spectrum-accordion-background-color-down: var(--spectrum-global-color-gray-300);
1516
}
1617

1718
.spectrum-Accordion-item {
@@ -25,20 +26,14 @@ governing permissions and limitations under the License.
2526
.spectrum-Accordion-itemHeader {
2627
color: var(--spectrum-alias-text-color);
2728

28-
&:hover {
29+
&:hover, &:focus-visible {
2930
color: var(--spectrum-alias-text-color-hover);
30-
3131
background-color: var(--spectrum-accordion-background-color-hover);
3232
}
33-
}
3433

35-
.spectrum-Accordion-item {
36-
&.is-expanded {
37-
.spectrum-Accordion-itemHeader {
38-
&:hover {
39-
background-color: transparent;
40-
}
41-
}
34+
&.is-pressed {
35+
color: var(--spectrum-accordion-text-color-down);
36+
background-color: var(--spectrum-accordion-background-color-down);
4237
}
4338
}
4439

@@ -54,8 +49,13 @@ governing permissions and limitations under the License.
5449
}
5550
@media (forced-colors: active) {
5651
.spectrum-Accordion-itemHeader {
52+
border: none;
5753
&:focus-visible {
58-
outline: 3px solid CanvasText;
54+
outline: 2px solid CanvasText;
5955
}
6056
}
57+
58+
.spectrum-Accordion-item.spectrum-Accordion-item--quiet {
59+
border: none
60+
}
6161
}
Loading

packages/@react-aria/disclosure/docs/useDisclosure.mdx

+17-3
Original file line numberDiff line numberDiff line change
@@ -73,18 +73,24 @@ This example displays a basic disclosure with a button that toggles the visibili
7373
import {useDisclosureState} from '@react-stately/disclosure';
7474
import {useDisclosure} from '@react-aria/disclosure';
7575
import {useButton} from '@react-aria/button';
76+
import {mergeProps, useFocusRing} from 'react-aria';
7677

7778
function Disclosure(props) {
7879
let state = useDisclosureState(props);
7980
let panelRef = React.useRef<HTMLDivElement | null>(null);
8081
let triggerRef = React.useRef<HTMLButtonElement | null>(null);
8182
let {buttonProps: triggerProps, panelProps} = useDisclosure(props, state, panelRef);
8283
let {buttonProps} = useButton(triggerProps, triggerRef);
84+
let {isFocusVisible, focusProps} = useFocusRing();
8385

8486
return (
8587
<div className="disclosure">
8688
<h3>
87-
<button className="trigger" ref={triggerRef} {...buttonProps}>
89+
<button
90+
className="trigger"
91+
ref={triggerRef}
92+
{...mergeProps(buttonProps, focusProps)}
93+
style={{outline: isFocusVisible? '2px solid dodgerblue' : 'none'}}>
8894
<svg viewBox="0 0 24 24">
8995
<path d="m8.25 4.5 7.5 7.5-7.5 7.5" />
9096
</svg>
@@ -109,6 +115,8 @@ function Disclosure(props) {
109115
<summary style={{fontWeight: 'bold'}}><ChevronRight size="S" /> Show CSS</summary>
110116

111117
```css
118+
@import "@react-aria/example-theme";
119+
112120
.disclosure {
113121
.trigger {
114122
background: none;
@@ -119,6 +127,7 @@ function Disclosure(props) {
119127
display: flex;
120128
align-items: center;
121129
gap: 8px;
130+
color: var(--text-color);
122131

123132
svg {
124133
rotate: 0deg;
@@ -187,7 +196,7 @@ A disclosure can be disabled with the `isDisabled` prop. This will disable the t
187196

188197
A disclosure group (i.e. accordion) is a set of disclosures where only one disclosure can be expanded at a time. The following example shows how to create a `DisclosureGroup` component with the `useDisclosureGroupState` hook. We'll also create a `DisclosureItem` component that uses the `DisclosureGroupState` context for managing its state.
189198

190-
```tsx example export=true
199+
```tsx example export=true render=false
191200
import {useDisclosureGroupState} from '@react-stately/disclosure';
192201
import {useId} from '@react-aria/utils';
193202

@@ -231,11 +240,16 @@ function DisclosureItem(props) {
231240
isDisabled
232241
}, state, panelRef);
233242
let {buttonProps} = useButton(triggerProps, triggerRef);
243+
let {isFocusVisible, focusProps} = useFocusRing();
234244

235245
return (
236246
<div className="disclosure">
237247
<h3>
238-
<button className="trigger" ref={triggerRef} {...buttonProps}>
248+
<button
249+
className="trigger"
250+
ref={triggerRef}
251+
{...mergeProps(buttonProps, focusProps)}
252+
style={{outline: isFocusVisible? '2px solid dodgerblue' : 'none'}}>
239253
<svg viewBox="0 0 24 24">
240254
<path d="m8.25 4.5 7.5 7.5-7.5 7.5" />
241255
</svg>

packages/@react-aria/disclosure/src/useDisclosure.ts

+3-3
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,8 @@ export function useDisclosure(props: AriaDisclosureProps, state: DisclosureState
102102
}
103103
},
104104
isDisabled,
105-
onKeyDown(e) {
106-
if (!isDisabled && (e.key === 'Enter' || e.key === ' ')) {
107-
e.preventDefault();
105+
onPressStart(e) {
106+
if (e.pointerType === 'keyboard' && !isDisabled) {
108107
state.toggle();
109108
}
110109
}
@@ -114,6 +113,7 @@ export function useDisclosure(props: AriaDisclosureProps, state: DisclosureState
114113
// This can be overridden at the panel element level.
115114
role: 'group',
116115
'aria-labelledby': triggerId,
116+
'aria-hidden': !state.isExpanded,
117117
hidden: supportsBeforeMatch ? true : !state.isExpanded
118118
}
119119
};

packages/@react-aria/disclosure/test/useDisclosure.test.ts

+6-8
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212
import {actHook as act, renderHook} from '@react-spectrum/test-utils-internal';
13-
import {KeyboardEvent, PressEvent} from '@react-types/shared';
13+
import {PressEvent} from '@react-types/shared';
1414
import {useDisclosure} from '../src/useDisclosure';
1515
import {useDisclosureState} from '@react-stately/disclosure';
1616

@@ -32,6 +32,7 @@ describe('useDisclosure', () => {
3232

3333
expect(buttonProps['aria-expanded']).toBe(false);
3434
expect(panelProps.hidden).toBe(true);
35+
expect(panelProps['aria-hidden']).toBe(true);
3536
});
3637

3738
it('should return correct aria attributes when expanded', () => {
@@ -46,7 +47,7 @@ describe('useDisclosure', () => {
4647
expect(panelProps.hidden).toBe(false);
4748
});
4849

49-
it('should handle expanding on press event', () => {
50+
it('should handle expanding on press event (with mouse)', () => {
5051
let {result} = renderHook(() => {
5152
let state = useDisclosureState({});
5253
let disclosure = useDisclosure({}, state, ref);
@@ -60,22 +61,19 @@ describe('useDisclosure', () => {
6061
expect(result.current.state.isExpanded).toBe(true);
6162
});
6263

63-
it('should handle expanding on keydown event', () => {
64+
it('should handle expanding on press start event (with keyboard)', () => {
6465
let {result} = renderHook(() => {
6566
let state = useDisclosureState({});
6667
let disclosure = useDisclosure({}, state, ref);
6768
return {state, disclosure};
6869
});
6970

70-
let preventDefault = jest.fn();
71-
let event = (e: Partial<KeyboardEvent>) => ({...e, preventDefault} as KeyboardEvent);
71+
let event = (e: Partial<PressEvent>) => (e as PressEvent);
7272

7373
act(() => {
74-
result.current.disclosure.buttonProps.onKeyDown?.(event({key: 'Enter', preventDefault}) as KeyboardEvent);
74+
result.current.disclosure.buttonProps.onPressStart?.(event({pointerType: 'keyboard'}) as PressEvent);
7575
});
7676

77-
expect(preventDefault).toHaveBeenCalledTimes(1);
78-
7977
expect(result.current.state.isExpanded).toBe(true);
8078
});
8179

packages/@react-spectrum/accordion/docs/Accordion.mdx

+12-1
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ export default Layout;
1313
import docs from 'docs:@react-spectrum/accordion';
1414
import {HeaderInfo, PropTable, TypeLink, PageDescription} from '@react-spectrum/docs';
1515
import packageData from '@react-spectrum/accordion/package.json';
16+
import ChevronRight from '@spectrum-icons/workflow/ChevronRight';
1617

1718
---
1819
category: Navigation
@@ -90,7 +91,7 @@ function ControlledExpansion() {
9091
}
9192
```
9293

93-
## Expansion
94+
## Expanded
9495

9596
By default, only one disclosure will be expanded at a time. To expand multiple disclosures, apply the `allowsMultipleExpanded` prop to Accordion.
9697

@@ -112,8 +113,18 @@ By default, only one disclosure will be expanded at a time. To expand multiple d
112113
```
113114
## Props
114115

116+
### Disclosure
117+
115118
<PropTable component={docs.exports.Accordion} links={docs.links} />
116119

120+
### Disclosure Panel
121+
122+
<PropTable component={docs.exports.DisclosurePanel} links={docs.links} />
123+
124+
### Disclosure Header
125+
126+
<PropTable component={docs.exports.DisclosureHeader} links={docs.links} />
127+
117128
## Visual Options
118129

119130
### Disabled

packages/@react-spectrum/accordion/src/Accordion.tsx

+19-9
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*/
1212

1313
import {AriaLabelingProps, DOMProps, DOMRef, StyleProps} from '@react-types/shared';
14-
import {Button, UNSTABLE_DisclosureGroup as DisclosureGroup, DisclosureGroupProps, DisclosurePanelProps, DisclosureProps, Heading, UNSTABLE_Disclosure as RACDisclosure, UNSTABLE_DisclosurePanel as RACDisclosurePanel} from 'react-aria-components';
14+
import {Button, DisclosureGroup, DisclosureGroupProps, DisclosurePanelProps, DisclosureProps, Heading, Disclosure as RACDisclosure, DisclosurePanel as RACDisclosurePanel} from 'react-aria-components';
1515
import ChevronLeftMedium from '@spectrum-icons/ui/ChevronLeftMedium';
1616
import ChevronRightMedium from '@spectrum-icons/ui/ChevronRightMedium';
1717
import {classNames, useDOMRef, useStyleProps} from '@react-spectrum/utils';
@@ -46,7 +46,7 @@ function Accordion(props: SpectrumAccordionProps, ref: DOMRef<HTMLDivElement>) {
4646
);
4747
}
4848

49-
export interface SpectrumDisclosureProps extends Omit<DisclosureProps, 'className' | 'style' | 'children'>, AriaLabelingProps {
49+
export interface SpectrumDisclosureProps extends Omit<DisclosureProps, 'className' | 'style' | 'children'>, AriaLabelingProps, StyleProps {
5050
/** Whether the Disclosure should be displayed with a quiet style. */
5151
isQuiet?: boolean,
5252
/** The contents of the disclosure. The first child should be the header, and the second child should be the panel. */
@@ -55,37 +55,45 @@ export interface SpectrumDisclosureProps extends Omit<DisclosureProps, 'classNam
5555

5656
function Disclosure(props: SpectrumDisclosureProps, ref: DOMRef<HTMLDivElement>) {
5757
props = useProviderProps(props);
58+
let {styleProps} = useStyleProps(props);
5859
let domRef = useDOMRef(ref);
5960
let accordionContext = React.useContext(InternalAccordionContext)!;
6061
return (
6162
<RACDisclosure
6263
{...props}
64+
{...styleProps}
6365
ref={domRef}
6466
className={({isExpanded, isDisabled}) => classNames(styles, 'spectrum-Accordion-item', {
6567
'spectrum-Accordion-item--quiet': accordionContext?.isQuiet ?? props.isQuiet,
6668
'is-expanded': isExpanded,
67-
'is-disabled': isDisabled
68-
})}>
69+
'is-disabled': isDisabled,
70+
'in-accordion': accordionContext != null
71+
}, styleProps.className)}>
6972
{props.children}
7073
</RACDisclosure>
7174
);
7275
}
7376

74-
export interface SpectrumDisclosurePanelProps extends Omit<DisclosurePanelProps, 'className' | 'style' | 'children'>, DOMProps, AriaLabelingProps {
77+
export interface SpectrumDisclosurePanelProps extends Omit<DisclosurePanelProps, 'className' | 'style' | 'children'>, DOMProps, AriaLabelingProps, StyleProps {
7578
/** The contents of the accordion panel. */
7679
children: React.ReactNode
7780
}
7881

7982
function DisclosurePanel(props: SpectrumDisclosurePanelProps, ref: DOMRef<HTMLDivElement>) {
83+
let {styleProps} = useStyleProps(props);
8084
let domRef = useDOMRef(ref);
8185
return (
82-
<RACDisclosurePanel ref={domRef} className={classNames(styles, 'spectrum-Accordion-itemContent')} {...props}>
86+
<RACDisclosurePanel
87+
ref={domRef}
88+
{...styleProps as Omit<React.HTMLAttributes<HTMLElement>, 'role'>}
89+
className={classNames(styles, 'spectrum-Accordion-itemContent', styleProps.className)}
90+
{...props}>
8391
{props.children}
8492
</RACDisclosurePanel>
8593
);
8694
}
8795

88-
export interface SpectrumDisclosureHeaderProps extends DOMProps, AriaLabelingProps {
96+
export interface SpectrumDisclosureHeaderProps extends DOMProps, AriaLabelingProps, StyleProps {
8997
/**
9098
* The heading level of the disclosure header.
9199
* @default 3
@@ -96,15 +104,17 @@ export interface SpectrumDisclosureHeaderProps extends DOMProps, AriaLabelingPro
96104
}
97105

98106
function DisclosureHeader(props: SpectrumDisclosureHeaderProps, ref: DOMRef<HTMLHeadingElement>) {
107+
let {styleProps} = useStyleProps(props);
99108
let {level = 3} = props;
100109
let {direction} = useLocale();
101110
let domRef = useDOMRef(ref);
102111
return (
103-
<Heading ref={domRef} level={level} className={classNames(styles, 'spectrum-Accordion-itemHeading')}>
112+
<Heading ref={domRef} level={level} {...styleProps} className={classNames(styles, 'spectrum-Accordion-itemHeading', styleProps.className)}>
104113
<Button
105114
slot="trigger"
106-
className={({isHovered, isFocusVisible}) => classNames(styles, 'spectrum-Accordion-itemHeader', {
115+
className={({isHovered, isFocusVisible, isPressed}) => classNames(styles, 'spectrum-Accordion-itemHeader', {
107116
'is-hovered': isHovered,
117+
'is-pressed': isPressed,
108118
'focus-ring': isFocusVisible
109119
})}>
110120
{direction === 'ltr' ? (

packages/@react-spectrum/s2/src/Accordion.tsx

+1-1
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
* governing permissions and limitations under the License.
1111
*/
1212

13-
import {ContextValue, UNSTABLE_DisclosureGroup as DisclosureGroup, DisclosureGroupProps, SlotProps} from 'react-aria-components';
13+
import {ContextValue, DisclosureGroup, DisclosureGroupProps, SlotProps} from 'react-aria-components';
1414
import {DisclosureContext} from './Disclosure';
1515
import {DOMProps, DOMRef, DOMRefValue} from '@react-types/shared';
1616
import {getAllowedOverrides, StylesPropWithHeight, UnsafeStyles} from './style-utils' with { type: 'macro' };

0 commit comments

Comments
 (0)