Skip to content

Commit 21cd7f0

Browse files
Slots updates (#2073)
Slots to override inline props on elements, the container knows better Fix some id bugs Co-authored-by: Danni <[email protected]>
1 parent 2cfa348 commit 21cd7f0

File tree

6 files changed

+214
-24
lines changed

6 files changed

+214
-24
lines changed

packages/@react-aria/utils/src/useId.ts

Lines changed: 18 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,9 @@ export function useId(defaultId?: string): string {
2525
isRendering.current = true;
2626
let [value, setValue] = useState(defaultId);
2727
let nextId = useRef(null);
28+
29+
let res = useSSRSafeId(value);
30+
2831
// don't memo this, we want it new each render so that the Effects always run
2932
let updateValue = (val) => {
3033
if (!isRendering.current) {
@@ -34,20 +37,26 @@ export function useId(defaultId?: string): string {
3437
}
3538
};
3639

40+
idsUpdaterMap.set(res, updateValue);
41+
3742
useLayoutEffect(() => {
3843
isRendering.current = false;
3944
}, [updateValue]);
4045

46+
useLayoutEffect(() => {
47+
let r = res;
48+
return () => {
49+
idsUpdaterMap.delete(r);
50+
};
51+
}, [res]);
52+
4153
useEffect(() => {
4254
let newId = nextId.current;
4355
if (newId) {
4456
setValue(newId);
4557
nextId.current = null;
4658
}
4759
}, [setValue, updateValue]);
48-
49-
let res = useSSRSafeId(value);
50-
idsUpdaterMap.set(res, updateValue);
5160
return res;
5261
}
5362

@@ -80,13 +89,16 @@ export function mergeIds(idA: string, idB: string): string {
8089
* if we can use it in places such as labelledby.
8190
*/
8291
export function useSlotId(): string {
83-
let [id, setId] = useState(useId());
92+
let id = useId();
93+
let [resolvedId, setResolvedId] = useState(id);
8494
useLayoutEffect(() => {
8595
let setCurr = idsUpdaterMap.get(id);
8696
if (setCurr && !document.getElementById(id)) {
87-
setId(null);
97+
setResolvedId(null);
98+
} else {
99+
setResolvedId(id);
88100
}
89101
}, [id]);
90102

91-
return id;
103+
return resolvedId;
92104
}

packages/@react-spectrum/buttongroup/src/ButtonGroup.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,10 @@ import {
2121
} from '@react-spectrum/utils';
2222
import {DOMRef} from '@react-types/shared';
2323
import {filterDOMProps} from '@react-aria/utils';
24+
import {Provider, useProvider, useProviderProps} from '@react-spectrum/provider';
2425
import React, {useCallback, useEffect, useRef} from 'react';
2526
import {SpectrumButtonGroupProps} from '@react-types/buttongroup';
2627
import styles from '@adobe/spectrum-css-temp/components/buttongroup/vars.css';
27-
import {useProvider, useProviderProps} from '@react-spectrum/provider';
2828

2929
function ButtonGroup(props: SpectrumButtonGroupProps, ref: DOMRef<HTMLDivElement>) {
3030
let {scale} = useProvider();
@@ -100,11 +100,12 @@ function ButtonGroup(props: SpectrumButtonGroupProps, ref: DOMRef<HTMLDivElement
100100
<SlotProvider
101101
slots={{
102102
button: {
103-
isDisabled,
104103
UNSAFE_className: classNames(styles, 'spectrum-ButtonGroup-Button')
105104
}
106105
}}>
107-
{children}
106+
<Provider isDisabled={isDisabled}>
107+
{children}
108+
</Provider>
108109
</SlotProvider>
109110
</div>
110111
);

packages/@react-spectrum/dialog/stories/DialogTrigger.stories.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -364,7 +364,7 @@ function render({width = 'auto', ...props}) {
364364
<ActionButton>Trigger</ActionButton>
365365
{(close) => (
366366
<Dialog>
367-
<Heading>The Heading</Heading>
367+
<Heading id="foo">The Heading</Heading>
368368
<Header>The Header</Header>
369369
<Divider />
370370
<Content><Text>Lorem ipsum dolor sit amet, consectetur adipiscing elit. Proin sit amet tristique risus. In sit amet suscipit lorem. Orci varius natoque penatibus et magnis dis parturient montes, nascetur ridiculus mus. In condimentum imperdiet metus non condimentum. Duis eu velit et quam accumsan tempus at id velit. Duis elementum elementum purus, id tempus mauris posuere a. Nunc vestibulum sapien pellentesque lectus commodo ornare.</Text></Content>

packages/@react-spectrum/text/src/Heading.tsx

Lines changed: 3 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,21 +11,13 @@
1111
*/
1212

1313
import {DOMRef} from '@react-types/shared';
14-
import {filterDOMProps, mergeProps} from '@react-aria/utils';
14+
import {filterDOMProps} from '@react-aria/utils';
1515
import {HeadingProps} from '@react-types/text';
1616
import React, {ElementType, forwardRef} from 'react';
1717
import {useDOMRef, useSlotProps, useStyleProps} from '@react-spectrum/utils';
1818

19-
const slotDOMProps = new Set(['aria-current']);
20-
2119
function Heading(props: HeadingProps, ref: DOMRef<HTMLHeadingElement>) {
22-
let slotProps = useSlotProps({}, 'heading');
23-
props = mergeProps(slotProps, props);
24-
25-
let domProps = mergeProps(
26-
filterDOMProps(props),
27-
filterDOMProps(slotProps, {propNames: slotDOMProps})
28-
);
20+
props = useSlotProps(props, 'heading');
2921

3022
let {
3123
children,
@@ -37,7 +29,7 @@ function Heading(props: HeadingProps, ref: DOMRef<HTMLHeadingElement>) {
3729
let HeadingTag = `h${level}` as ElementType;
3830

3931
return (
40-
<HeadingTag {...domProps} {...styleProps} ref={domRef}>
32+
<HeadingTag {...filterDOMProps(otherProps)} {...styleProps} ref={domRef}>
4133
{children}
4234
</HeadingTag>
4335
);

packages/@react-spectrum/utils/src/Slots.tsx

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,10 +19,11 @@ interface SlotProps {
1919

2020
let SlotContext = React.createContext(null);
2121

22-
export function useSlotProps<T>(props: T, defaultSlot?: string): T {
22+
export function useSlotProps<T>(props: T & {id?: string}, defaultSlot?: string): T {
2323
let slot = (props as SlotProps).slot || defaultSlot;
2424
let {[slot]: slotProps = {}} = useContext(SlotContext) || {};
25-
return mergeProps(slotProps, props);
25+
26+
return mergeProps(props, mergeProps(slotProps, {id: props.id}));
2627
}
2728

2829
export function cssModuleToSlots(cssModule) {
@@ -37,7 +38,7 @@ export function SlotProvider(props) {
3738
let {slots = {}, children} = props;
3839

3940
// Merge props for each slot from parent context and props
40-
let value = useMemo(() =>
41+
let value = useMemo(() =>
4142
Object.keys(parentSlots)
4243
.concat(Object.keys(slots))
4344
.reduce((o, p) => ({
Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
/*
2+
* Copyright 2020 Adobe. All rights reserved.
3+
* This file is licensed to you under the Apache License, Version 2.0 (the "License");
4+
* you may not use this file except in compliance with the License. You may obtain a copy
5+
* of the License at http://www.apache.org/licenses/LICENSE-2.0
6+
*
7+
* Unless required by applicable law or agreed to in writing, software distributed under
8+
* the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTATIONS
9+
* OF ANY KIND, either express or implied. See the License for the specific language
10+
* governing permissions and limitations under the License.
11+
*/
12+
13+
import React, {useRef} from 'react';
14+
import {render} from '@testing-library/react';
15+
import {SlotProvider, useSlotProps} from '../';
16+
import {triggerPress} from '@react-spectrum/test-utils';
17+
import {useId, useSlotId} from '@react-aria/utils';
18+
import {usePress} from '@react-aria/interactions';
19+
20+
21+
describe('Slots', function () {
22+
let results = {};
23+
24+
afterEach(() => {
25+
results = {};
26+
});
27+
28+
function Component(props) {
29+
results = useSlotProps(props, 'slotname');
30+
props = results;
31+
let ref = useRef();
32+
let {pressProps} = usePress({onPress: props.onPress, ref});
33+
let id = useId(props.id);
34+
return <button id={id} {...pressProps} ref={ref}>push me</button>;
35+
}
36+
37+
it('sets props', function () {
38+
let slots = {
39+
slotname: {UNSAFE_className: 'foo', isDisabled: true, isQuiet: true}
40+
};
41+
render(
42+
<SlotProvider slots={slots}>
43+
<Component />
44+
</SlotProvider>
45+
);
46+
expect(results).toMatchObject({
47+
UNSAFE_className: 'foo',
48+
isDisabled: true,
49+
isQuiet: true
50+
});
51+
});
52+
53+
it('overrides local props', function () {
54+
let slots = {
55+
slotname: {UNSAFE_className: 'foo', isDisabled: false, isQuiet: false, label: null}
56+
};
57+
render(
58+
<SlotProvider slots={slots}>
59+
<Component UNSAFE_className="bar" isDisabled isQuiet label="boop" />
60+
</SlotProvider>
61+
);
62+
expect(results).toMatchObject({
63+
UNSAFE_className: expect.stringMatching(/(foo bar|bar foo)/),
64+
isDisabled: false,
65+
isQuiet: false,
66+
label: null
67+
});
68+
});
69+
70+
it('undefined does not override local props', function () {
71+
let slots = {
72+
slotname: {label: undefined}
73+
};
74+
render(
75+
<SlotProvider slots={slots}>
76+
<Component label="boop" />
77+
</SlotProvider>
78+
);
79+
expect(results).toMatchObject({
80+
label: 'boop'
81+
});
82+
});
83+
84+
it('chains functions', function () {
85+
let onPress = jest.fn();
86+
let onPressUser = jest.fn();
87+
let slots = {
88+
slotname: {onPress}
89+
};
90+
let {getByRole} = render(
91+
<SlotProvider slots={slots}>
92+
<Component label="boop" onPress={onPressUser} />
93+
</SlotProvider>
94+
);
95+
triggerPress(getByRole('button'));
96+
expect(onPress).toHaveBeenCalledTimes(1);
97+
expect(onPressUser).toHaveBeenCalledTimes(1);
98+
});
99+
100+
it('lets users set their own id', function () {
101+
let slots = {
102+
slotname: {id: 'foo'}
103+
};
104+
render(
105+
<SlotProvider slots={slots}>
106+
<Component label="boop" id="bar" />
107+
</SlotProvider>
108+
);
109+
expect(results).toMatchObject({id: 'bar'});
110+
});
111+
112+
it('lets users set their own id when used in conjunction with useId even if a default is passed to useId', function () {
113+
function SlotsUseId(props) {
114+
let id = useId(props.id);
115+
return (
116+
<>
117+
<div role="presentation" aria-controls={id}>nowhere</div>
118+
<SlotProvider slots={{slotname: {...props.slots, id}}}>
119+
<Component id="bar" />
120+
</SlotProvider>
121+
</>
122+
);
123+
}
124+
let {getByRole} = render(<SlotsUseId id="foo" />);
125+
expect(results).toMatchObject({id: 'bar'}); // we've merged with the user provided id
126+
expect(getByRole('presentation')).toHaveAttribute('aria-controls', 'bar');
127+
});
128+
129+
it('lets users set their own id when used in conjunction with useId', function () {
130+
function SlotsUseId(props) {
131+
let id = useId();
132+
return (
133+
<>
134+
<div role="presentation" aria-controls={id}>nowhere</div>
135+
<SlotProvider slots={{slotname: {...props.slots, id}}}>
136+
<Component id="bar" />
137+
</SlotProvider>
138+
</>
139+
);
140+
}
141+
let {getByRole} = render(<SlotsUseId />);
142+
expect(results).toMatchObject({id: 'bar'}); // we've merged with the user provided id
143+
expect(getByRole('presentation')).toHaveAttribute('aria-controls', 'bar');
144+
});
145+
146+
it('lets users set their own id when used in conjunction with useSlotId', function () {
147+
function SlotsUseSlotId() {
148+
let id = useSlotId();
149+
return (
150+
<>
151+
<div role="presentation" aria-controls={id}>nowhere</div>
152+
<SlotProvider slots={{slotname: {id}}}>
153+
<Component id="bar" />
154+
</SlotProvider>
155+
</>
156+
);
157+
}
158+
let {getByRole} = render(<SlotsUseSlotId />);
159+
expect(results).toMatchObject({id: 'bar'}); // we've merged with the user provided id
160+
expect(getByRole('presentation')).toHaveAttribute('aria-controls', 'bar');
161+
});
162+
163+
it('multiple useIds can be used together and resolve to the one used on the component', function () {
164+
let id = '';
165+
function SlotsUseSlotId() {
166+
let id1 = useId();
167+
let id2 = useId();
168+
id = useRef(id2).current;
169+
170+
return (
171+
<>
172+
<div role="presentation" aria-controls={id1}>nowhere</div>
173+
<SlotProvider slots={{slotname: {id: id1}}}>
174+
<Component id={id2} />
175+
</SlotProvider>
176+
</>
177+
);
178+
}
179+
let {getByRole} = render(<SlotsUseSlotId />);
180+
expect(results).toMatchObject({id}); // we've merged with the user provided id
181+
expect(getByRole('presentation')).toHaveAttribute('aria-controls', id);
182+
});
183+
184+
});

0 commit comments

Comments
 (0)