Skip to content

Commit

Permalink
Rename onOpenChange to onValueChange and misc fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
mj12albert committed Oct 9, 2024
1 parent d00847d commit b769cbb
Show file tree
Hide file tree
Showing 11 changed files with 53 additions and 47 deletions.
10 changes: 5 additions & 5 deletions docs/data/components/accordion/accordion.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -113,13 +113,13 @@ When uncontrolled, use the `defaultValue` prop to set the initial state of the a

### Controlled

When controlled, pass the `value` and `onOpenChange` props to `Accordion.Root`:
When controlled, pass the `value` and `onValueChange` props to `Accordion.Root`:

```tsx
const [value, setValue] = React.useState(['a']);

return (
<Accordion.Root value={value} onOpenChange={setValue}>
<Accordion.Root value={value} onValueChange={setValue}>
<Accordion.Item value="a">
<Accordion.Header>
<Accordion.Trigger>Toggle one</Accordion.Trigger>
Expand Down Expand Up @@ -153,14 +153,14 @@ Use controlled mode to always keep one `Item` open:
```tsx
const [value, setValue] = React.useState([0]);

const handleOpenChange = (newValue) => {
const handleValueChange = (newValue) => {
if (newValue.length > 0) {
setValue(newValue);
}
};

return (
<Accordion.Root value={value} onOpenChange={handleOpenChange}>
<Accordion.Root value={value} onValueChange={handleValueChange}>
{/* subcomponents */}
</Accordion.Root>
);
Expand Down Expand Up @@ -295,7 +295,7 @@ When using external libraries for animation, for example `framer-motion`, be awa
function App() {
const [value, setValue] = useState([0]);
return (
<Accordion.Root value={value} onOpenChange={setValue}>
<Accordion.Root value={value} onValueChange={setValue}>
<Accordion.Item>
<Accordion.Header>
<Accordion.Trigger>Toggle</Accordion.Trigger>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const accordionRootContextValue: Accordion.Root.Context = {
animated: false,
direction: 'ltr',
disabled: false,
handleOpenChange() {},
handleValueChange: NOOP,
hiddenUntilFound: false,
orientation: 'vertical',
ownerState: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const accordionRootContextValue: Accordion.Root.Context = {
animated: false,
direction: 'ltr',
disabled: false,
handleOpenChange: NOOP,
handleValueChange: NOOP,
hiddenUntilFound: false,
orientation: 'vertical',
ownerState: {
Expand Down
4 changes: 2 additions & 2 deletions packages/mui-base/src/Accordion/Item/AccordionItem.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ const AccordionItem = React.forwardRef(function AccordionItem(
const {
animated,
disabled: contextDisabled,
handleOpenChange,
handleValueChange,
ownerState: rootOwnerState,
value: openValues,
} = useAccordionRootContext();
Expand All @@ -69,7 +69,7 @@ const AccordionItem = React.forwardRef(function AccordionItem(
}, [openValues, value]);

const onOpenChange = useEventCallback((nextOpen: boolean) => {
handleOpenChange(value, nextOpen);
handleValueChange(value, nextOpen);
onOpenChangeProp?.(nextOpen);
});

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const accordionRootContextValue: Accordion.Root.Context = {
animated: false,
direction: 'ltr',
disabled: false,
handleOpenChange: NOOP,
handleValueChange: NOOP,
hiddenUntilFound: false,
orientation: 'vertical',
ownerState: {
Expand Down
6 changes: 6 additions & 0 deletions packages/mui-base/src/Accordion/Panel/AccordionPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const AccordionPanel = React.forwardRef(function AccordionPanel(
hiddenUntilFound: hiddenUntilFoundProp,
id: idProp,
render,
style: styleProp,
...otherProps
} = props;

Expand Down Expand Up @@ -63,6 +64,7 @@ const AccordionPanel = React.forwardRef(function AccordionPanel(
style: {
'--accordion-content-height': height ? `${height}px` : undefined,
'--accordion-content-width': width ? `${width}px` : undefined,
...styleProp,
},
},
customStyleHookMapping: accordionStyleHookMapping,
Expand Down Expand Up @@ -106,4 +108,8 @@ AccordionPanel.propTypes /* remove-proptypes */ = {
* A function to customize rendering of the component.
*/
render: PropTypes.oneOfType([PropTypes.element, PropTypes.func]),
/**
* @ignore
*/
style: PropTypes.object,
} as any;
44 changes: 22 additions & 22 deletions packages/mui-base/src/Accordion/Root/AccordionRoot.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -710,12 +710,12 @@ describe('<Accordion.Root />', () => {
});
});

describe('prop: onOpenChange', () => {
describe('prop: onValueChange', () => {
it('default item value', async () => {
const onOpenChange = spy();
const onValueChange = spy();

const { getByTestId, user } = await render(
<Accordion.Root data-testid="root" animated={false} onOpenChange={onOpenChange}>
<Accordion.Root data-testid="root" animated={false} onValueChange={onValueChange}>
<Accordion.Item>
<Accordion.Header>
<Accordion.Trigger data-testid="trigger1">Trigger 1</Accordion.Trigger>
Expand All @@ -734,24 +734,24 @@ describe('<Accordion.Root />', () => {
const trigger1 = getByTestId('trigger1');
const trigger2 = getByTestId('trigger2');

expect(onOpenChange.callCount).to.equal(0);
expect(onValueChange.callCount).to.equal(0);

await user.pointer({ keys: '[MouseLeft]', target: trigger1 });

expect(onOpenChange.callCount).to.equal(1);
expect(onOpenChange.args[0][0]).to.deep.equal([0]);
expect(onValueChange.callCount).to.equal(1);
expect(onValueChange.args[0][0]).to.deep.equal([0]);

await user.pointer({ keys: '[MouseLeft]', target: trigger2 });

expect(onOpenChange.callCount).to.equal(2);
expect(onOpenChange.args[1][0]).to.deep.equal([0, 1]);
expect(onValueChange.callCount).to.equal(2);
expect(onValueChange.args[1][0]).to.deep.equal([0, 1]);
});

it('custom item value', async () => {
const onOpenChange = spy();
const onValueChange = spy();

const { getByTestId, user } = await render(
<Accordion.Root data-testid="root" animated={false} onOpenChange={onOpenChange}>
<Accordion.Root data-testid="root" animated={false} onValueChange={onValueChange}>
<Accordion.Item value="one">
<Accordion.Header>
<Accordion.Trigger data-testid="trigger1">Trigger 1</Accordion.Trigger>
Expand All @@ -770,27 +770,27 @@ describe('<Accordion.Root />', () => {
const trigger1 = getByTestId('trigger1');
const trigger2 = getByTestId('trigger2');

expect(onOpenChange.callCount).to.equal(0);
expect(onValueChange.callCount).to.equal(0);

await user.pointer({ keys: '[MouseLeft]', target: trigger2 });

expect(onOpenChange.callCount).to.equal(1);
expect(onOpenChange.args[0][0]).to.deep.equal(['two']);
expect(onValueChange.callCount).to.equal(1);
expect(onValueChange.args[0][0]).to.deep.equal(['two']);

await user.pointer({ keys: '[MouseLeft]', target: trigger1 });

expect(onOpenChange.callCount).to.equal(2);
expect(onOpenChange.args[1][0]).to.deep.equal(['two', 'one']);
expect(onValueChange.callCount).to.equal(2);
expect(onValueChange.args[1][0]).to.deep.equal(['two', 'one']);
});

it('openMultiple is false', async () => {
const onOpenChange = spy();
const onValueChange = spy();

const { getByTestId, user } = await render(
<Accordion.Root
data-testid="root"
animated={false}
onOpenChange={onOpenChange}
onValueChange={onValueChange}
openMultiple={false}
>
<Accordion.Item value="one">
Expand All @@ -811,17 +811,17 @@ describe('<Accordion.Root />', () => {
const trigger1 = getByTestId('trigger1');
const trigger2 = getByTestId('trigger2');

expect(onOpenChange.callCount).to.equal(0);
expect(onValueChange.callCount).to.equal(0);

await user.pointer({ keys: '[MouseLeft]', target: trigger1 });

expect(onOpenChange.callCount).to.equal(1);
expect(onOpenChange.args[0][0]).to.deep.equal(['one']);
expect(onValueChange.callCount).to.equal(1);
expect(onValueChange.args[0][0]).to.deep.equal(['one']);

await user.pointer({ keys: '[MouseLeft]', target: trigger2 });

expect(onOpenChange.callCount).to.equal(2);
expect(onOpenChange.args[1][0]).to.deep.equal(['two']);
expect(onValueChange.callCount).to.equal(2);
expect(onValueChange.args[1][0]).to.deep.equal(['two']);
});
});
});
6 changes: 3 additions & 3 deletions packages/mui-base/src/Accordion/Root/AccordionRoot.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ const AccordionRoot = React.forwardRef(function AccordionRoot(
disabled = false,
hiddenUntilFound = false,
loop,
onOpenChange,
onValueChange,
openMultiple = true,
orientation,
value,
Expand All @@ -54,7 +54,7 @@ const AccordionRoot = React.forwardRef(function AccordionRoot(
defaultValue,
loop,
orientation,
onOpenChange,
onValueChange,
openMultiple,
value,
});
Expand Down Expand Up @@ -168,7 +168,7 @@ AccordionRoot.propTypes /* remove-proptypes */ = {
* Callback fired when an Accordion section is opened or closed.
* The value representing the involved section is provided as an argument.
*/
onOpenChange: PropTypes.func,
onValueChange: PropTypes.func,
/**
* Whether multiple Accordion sections can be opened at the same time
* @default true
Expand Down
20 changes: 10 additions & 10 deletions packages/mui-base/src/Accordion/Root/useAccordionRoot.ts
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ export function useAccordionRoot(
disabled = false,
direction = 'ltr',
loop = true,
onOpenChange = NOOP,
onValueChange = NOOP,
orientation = 'vertical',
openMultiple = true,
value: valueParam,
Expand All @@ -59,24 +59,24 @@ export function useAccordionRoot(
state: 'value',
});

const handleOpenChange = React.useCallback(
const handleValueChange = React.useCallback(
(newValue: number | string, nextOpen: boolean) => {
if (!openMultiple) {
const nextValue = value[0] === newValue ? [] : [newValue];
setValue(nextValue);
onOpenChange(nextValue);
onValueChange(nextValue);
} else if (nextOpen) {
const nextOpenValues = value.slice();
nextOpenValues.push(newValue);
setValue(nextOpenValues);
onOpenChange(nextOpenValues);
onValueChange(nextOpenValues);
} else {
const nextOpenValues = value.filter((v) => v !== newValue);
setValue(nextOpenValues);
onOpenChange(nextOpenValues);
onValueChange(nextOpenValues);
}
},
[onOpenChange, openMultiple, setValue, value],
[onValueChange, openMultiple, setValue, value],
);

const getRootProps = React.useCallback(
Expand Down Expand Up @@ -173,7 +173,7 @@ export function useAccordionRoot(
animated,
direction,
disabled,
handleOpenChange,
handleValueChange,
orientation,
value,
}),
Expand All @@ -183,7 +183,7 @@ export function useAccordionRoot(
animated,
direction,
disabled,
handleOpenChange,
handleValueChange,
orientation,
value,
],
Expand Down Expand Up @@ -233,7 +233,7 @@ export namespace useAccordionRoot {
* Callback fired when an Accordion section is opened or closed.
* The value representing the involved section is provided as an argument.
*/
onOpenChange?: (value: Value) => void;
onValueChange?: (value: Value) => void;
/**
* Whether multiple Accordion sections can be opened at the same time
* @default true
Expand All @@ -256,7 +256,7 @@ export namespace useAccordionRoot {
* The disabled state of the Accordion
*/
disabled: boolean;
handleOpenChange: (value: number | string, nextOpen: boolean) => void;
handleValueChange: (value: number | string, nextOpen: boolean) => void;
orientation: Orientation;
/**
* The open state of the Accordion represented by an array of the values
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ const accordionRootContextValue: Accordion.Root.Context = {
animated: false,
direction: 'ltr',
disabled: false,
handleOpenChange: NOOP,
handleValueChange: NOOP,
hiddenUntilFound: false,
orientation: 'vertical',
ownerState: {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function useCollapsibleTrigger(
): useCollapsibleTrigger.ReturnValue {
const { contentId, disabled, id, open, rootRef: externalRef, setOpen } = parameters;

const { getRootProps: getButtonProps, rootRef: buttonRef } = useButton({
const { getButtonProps, buttonRef } = useButton({
disabled,
focusableWhenDisabled: true,
type: 'button',
Expand Down

0 comments on commit b769cbb

Please sign in to comment.