Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ExpansionPanel] Use context instead of cloneElement #18085

Merged
merged 4 commits into from
Oct 31, 2019
Merged
Show file tree
Hide file tree
Changes from 2 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
9 changes: 9 additions & 0 deletions packages/material-ui/src/ExpansionPanel/Context.js
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 Context = React.createContext({});
eps1lon marked this conversation as resolved.
Show resolved Hide resolved

export default Context;
eps1lon marked this conversation as resolved.
Show resolved Hide resolved
31 changes: 18 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 Context from './Context';

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,7 @@ const ExpansionPanel = React.forwardRef(function ExpansionPanel(props, ref) {
square={square}
{...other}
>
{React.cloneElement(summary, {
disabled,
expanded,
onChange: handleChange,
})}
<Context.Provider value={contextValue}>{summary}</Context.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
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/Context';

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