Skip to content

Commit

Permalink
[ExpansionPanel] Use context instead of cloneElement (#18085)
Browse files Browse the repository at this point in the history
  • Loading branch information
eps1lon committed Oct 31, 2019
1 parent c28697d commit 3748723
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 40 deletions.
33 changes: 20 additions & 13 deletions packages/material-ui/src/ExpansionPanel/ExpansionPanel.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { chainPropTypes } from '@material-ui/utils';
import Collapse from '../Collapse';
import Paper from '../Paper';
import withStyles from '../styles/withStyles';
import ExpansionPanelContext from './ExpansionPanelContext';

export const styles = theme => {
const transition = {
Expand Down Expand Up @@ -115,17 +116,25 @@ const ExpansionPanel = React.forwardRef(function ExpansionPanel(props, ref) {
}, [expandedProp, isControlled]);
}

const handleChange = event => {
if (!isControlled) {
setExpandedState(!expanded);
}
const handleChange = React.useCallback(
event => {
if (!isControlled) {
setExpandedState(!expanded);
}

if (onChange) {
onChange(event, !expanded);
}
};
if (onChange) {
onChange(event, !expanded);
}
},
[expanded, isControlled, onChange],
);

const [summary, ...children] = React.Children.toArray(childrenProp);
const contextValue = React.useMemo(() => ({ expanded, disabled, toggle: handleChange }), [
expanded,
disabled,
handleChange,
]);

return (
<Paper
Expand All @@ -142,11 +151,9 @@ const ExpansionPanel = React.forwardRef(function ExpansionPanel(props, ref) {
square={square}
{...other}
>
{React.cloneElement(summary, {
disabled,
expanded,
onChange: handleChange,
})}
<ExpansionPanelContext.Provider value={contextValue}>
{summary}
</ExpansionPanelContext.Provider>
<TransitionComponent in={expanded} timeout="auto" {...TransitionProps}>
<div aria-labelledby={summary.props.id} id={summary.props['aria-controls']} role="region">
{children}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import React from 'react';

/**
* @ignore - internal component.
* @type {React.Context<{} | {expanded: boolean, disabled: boolean, toggle: () => void}>}
*/
const ExpansionPanelContext = React.createContext({});

export default ExpansionPanelContext;
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,8 @@ export type ExpansionPanelSummaryTypeMap<
D extends React.ElementType = 'div'
> = ExtendButtonBaseTypeMap<{
props: P & {
disabled?: boolean;
expanded?: boolean;
expandIcon?: React.ReactNode;
IconButtonProps?: Partial<IconButtonProps>;
onChange?: React.ReactEventHandler<{}>;
};
defaultComponent: D;
classKey: ExpansionPanelSummaryClassKey;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import clsx from 'clsx';
import ButtonBase from '../ButtonBase';
import IconButton from '../IconButton';
import withStyles from '../styles/withStyles';
import ExpansionPanelContext from '../ExpansionPanel/ExpansionPanelContext';

export const styles = theme => {
const transition = {
Expand Down Expand Up @@ -68,12 +69,9 @@ const ExpansionPanelSummary = React.forwardRef(function ExpansionPanelSummary(pr
children,
classes,
className,
disabled = false,
expanded,
expandIcon,
IconButtonProps,
onBlur,
onChange,
onClick,
onFocusVisible,
...other
Expand All @@ -94,10 +92,11 @@ const ExpansionPanelSummary = React.forwardRef(function ExpansionPanelSummary(pr
onBlur(event);
}
};
// TODO: Remove in v5 and use onClick in ExpansionPanel.js

const { disabled = false, expanded, toggle } = React.useContext(ExpansionPanelContext);
const handleChange = event => {
if (onChange) {
onChange(event);
if (toggle) {
toggle(event);
}
if (onClick) {
onClick(event);
Expand Down Expand Up @@ -160,16 +159,6 @@ ExpansionPanelSummary.propTypes = {
* @ignore
*/
className: PropTypes.string,
/**
* @ignore
* If `true`, the summary will be displayed in a disabled state.
*/
disabled: PropTypes.bool,
/**
* @ignore
* If `true`, expands the summary, otherwise collapse it.
*/
expanded: PropTypes.bool,
/**
* The icon to display as the expand indicator.
*/
Expand All @@ -182,10 +171,6 @@ ExpansionPanelSummary.propTypes = {
* @ignore
*/
onBlur: PropTypes.func,
/**
* @ignore
*/
onChange: PropTypes.func,
/**
* @ignore
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { spy } from 'sinon';
import { createMount, getClasses } from '@material-ui/core/test-utils';
import describeConformance from '../test-utils/describeConformance';
import { createClientRender, fireEvent } from 'test/utils/createClientRender';
import ExpansionPanel from '../ExpansionPanel';
import ExpansionPanelSummary from './ExpansionPanelSummary';
import ButtonBase from '../ButtonBase';

Expand All @@ -13,6 +14,7 @@ describe('<ExpansionPanelSummary />', () => {
const render = createClientRender({ strict: true });

before(() => {
// requires mocking the TransitionComponent of `ExpansionPanel`
mount = createMount({ strict: true });
classes = getClasses(<ExpansionPanelSummary />);
});
Expand All @@ -33,13 +35,21 @@ describe('<ExpansionPanelSummary />', () => {
});

it('when disabled should have disabled class', () => {
const { getByRole } = render(<ExpansionPanelSummary disabled />);
const { getByRole } = render(
<ExpansionPanel disabled TransitionComponent={({ children }) => children}>
<ExpansionPanelSummary />
</ExpansionPanel>,
);

expect(getByRole('button')).to.have.class(classes.disabled);
});

it('when expanded adds the expanded class to the button and expandIcon', () => {
const { container, getByRole } = render(<ExpansionPanelSummary expanded expandIcon="expand" />);
const { container, getByRole } = render(
<ExpansionPanel expanded TransitionComponent={({ children }) => children}>
<ExpansionPanelSummary expandIcon="expand" />
</ExpansionPanel>,
);

const button = getByRole('button');
expect(button).to.have.class(classes.expanded);
Expand Down Expand Up @@ -93,9 +103,13 @@ describe('<ExpansionPanelSummary />', () => {
expect(handleClick.callCount).to.equal(1);
});

it('calls onChange when clicking', () => {
it('fires onChange of the ExpansionPanel if clicked', () => {
const handleChange = spy();
const { getByRole } = render(<ExpansionPanelSummary onChange={handleChange} />);
const { getByRole } = render(
<ExpansionPanel onChange={handleChange} TransitionComponent={({ children }) => children}>
<ExpansionPanelSummary />
</ExpansionPanel>,
);

getByRole('button').click();

Expand Down

0 comments on commit 3748723

Please sign in to comment.