Skip to content

Commit

Permalink
[Accordion] Deprecate classes.focused
Browse files Browse the repository at this point in the history
  • Loading branch information
oliviertassinari committed Dec 20, 2020
1 parent 411cc61 commit e000cf3
Show file tree
Hide file tree
Showing 5 changed files with 39 additions and 61 deletions.
5 changes: 3 additions & 2 deletions docs/pages/api-docs/accordion-summary.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ The `MuiAccordionSummary` name can be used for providing [default props](/custom
| <span class="prop-name">children</span> | <span class="prop-type">node</span> | | The content of the accordion summary. |
| <span class="prop-name">classes</span> | <span class="prop-type">object</span> | | Override or extend the styles applied to the component. See [CSS API](#css) below for more details. |
| <span class="prop-name">expandIcon</span> | <span class="prop-type">node</span> | | The icon to display as the expand indicator. |
| <span class="prop-name">IconButtonProps</span> | <span class="prop-type">object</span> | | Props applied to the `IconButton` element wrapping the expand icon. |
| <span class="prop-name">focusVisibleClassName</span> | <span class="prop-type">string</span> | | This prop can help a person know which element has the keyboard focus. The class name will be applied when the element gain the focus through a keyboard interaction. It's a polyfill for the [CSS :focus-visible selector](https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo). The rationale for using this feature [is explained here](https://github.com/WICG/focus-visible/blob/master/explainer.md). A [polyfill can be used](https://github.com/WICG/focus-visible) to apply a `focus-visible` class to other components if needed. |
| <span class="prop-name">IconButtonProps</span> | <span class="prop-type">object</span> | <span class="prop-default">{}</span> | Props applied to the `IconButton` element wrapping the expand icon. |

The `ref` is forwarded to the root element.

Expand All @@ -43,7 +44,7 @@ Any other props supplied will be provided to the root element ([ButtonBase](/api
|:-----|:-------------|:------------|
| <span class="prop-name">root</span> | <span class="prop-name">.MuiAccordionSummary-root</span> | Styles applied to the root element.
| <span class="prop-name">expanded</span> | <span class="prop-name">.Mui-expanded</span> | Pseudo-class applied to the root element, children wrapper element and `IconButton` component if `expanded={true}`.
| <span class="prop-name">focused</span> | <span class="prop-name">.Mui-focused</span> | Pseudo-class applied to the root element if `focused={true}`.
| <span class="prop-name">focusVisible</span> | <span class="prop-name">.Mui-focusVisible</span> | Pseudo-class applied to the ButtonBase root element if the button is keyboard focused.
| <span class="prop-name">disabled</span> | <span class="prop-name">.Mui-disabled</span> | Pseudo-class applied to the root element if `disabled={true}`.
| <span class="prop-name">content</span> | <span class="prop-name">.MuiAccordionSummary-content</span> | Styles applied to the children wrapper element.
| <span class="prop-name">expandIcon</span> | <span class="prop-name">.MuiAccordionSummary-expandIcon</span> | Styles applied to the `IconButton` component when `expandIcon` is supplied.
Expand Down
12 changes: 11 additions & 1 deletion packages/material-ui/src/Accordion/Accordion.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,17 @@ Accordion.propTypes = {
* Override or extend the styles applied to the component.
* See [CSS API](#css) below for more details.
*/
classes: PropTypes.object,
classes: chainPropTypes(PropTypes.object, (props) => {
if (props.classes && props.classes.focused) {
throw new Error([
'Material-UI: the classes.focused key is deprecated.',
'Use `classes.focusedVisible` instead',
'The name of the pseudo-class was changed for consistency.',
]).join('\n');
}

return null;
}),
/**
* @ignore
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export type AccordionSummaryClassKey =
| 'root'
| 'expanded'
| 'focused'
| 'focusVisible'
| 'disabled'
| 'content'
| 'expandIcon';
Expand Down
48 changes: 15 additions & 33 deletions packages/material-ui/src/AccordionSummary/AccordionSummary.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export const styles = (theme) => {
'&$expanded': {
minHeight: 64,
},
'&$focused': {
'&$focusVisible': {
backgroundColor: theme.palette.action.focus,
},
'&$disabled': {
Expand All @@ -34,8 +34,8 @@ export const styles = (theme) => {
},
/* Pseudo-class applied to the root element, children wrapper element and `IconButton` component if `expanded={true}`. */
expanded: {},
/* Pseudo-class applied to the root element if `focused={true}`. */
focused: {},
/* Pseudo-class applied to the ButtonBase root element if the button is keyboard focused. */
focusVisible: {},
/* Pseudo-class applied to the root element if `disabled={true}`. */
disabled: {},
/* Styles applied to the children wrapper element. */
Expand Down Expand Up @@ -71,29 +71,12 @@ const AccordionSummary = React.forwardRef(function AccordionSummary(props, ref)
classes,
className,
expandIcon,
IconButtonProps,
onBlur,
focusVisibleClassName,
IconButtonProps = {},
onClick,
onFocusVisible,
...other
} = props;

const [focusedState, setFocusedState] = React.useState(false);
const handleFocusVisible = (event) => {
setFocusedState(true);

if (onFocusVisible) {
onFocusVisible(event);
}
};
const handleBlur = (event) => {
setFocusedState(false);

if (onBlur) {
onBlur(event);
}
};

const { disabled = false, expanded, toggle } = React.useContext(AccordionContext);
const handleChange = (event) => {
if (toggle) {
Expand All @@ -116,12 +99,10 @@ const AccordionSummary = React.forwardRef(function AccordionSummary(props, ref)
{
[classes.disabled]: disabled,
[classes.expanded]: expanded,
[classes.focused]: focusedState,
},
className,
)}
onFocusVisible={handleFocusVisible}
onBlur={handleBlur}
focusVisibleClassName={clsx(classes.focusVisible, classes.focused, focusVisibleClassName)}
onClick={handleChange}
ref={ref}
{...other}
Expand Down Expand Up @@ -169,21 +150,22 @@ AccordionSummary.propTypes = {
*/
expandIcon: PropTypes.node,
/**
* Props applied to the `IconButton` element wrapping the expand icon.
* This prop can help a person know which element has the keyboard focus.
* The class name will be applied when the element gain the focus through a keyboard interaction.
* It's a polyfill for the [CSS :focus-visible selector](https://drafts.csswg.org/selectors-4/#the-focus-visible-pseudo).
* The rationale for using this feature [is explained here](https://github.com/WICG/focus-visible/blob/master/explainer.md).
* A [polyfill can be used](https://github.com/WICG/focus-visible) to apply a `focus-visible` class to other components
* if needed.
*/
IconButtonProps: PropTypes.object,
focusVisibleClassName: PropTypes.string,
/**
* @ignore
* Props applied to the `IconButton` element wrapping the expand icon.
*/
onBlur: PropTypes.func,
IconButtonProps: PropTypes.object,
/**
* @ignore
*/
onClick: PropTypes.func,
/**
* @ignore
*/
onFocusVisible: PropTypes.func,
};

export default withStyles(styles, { name: 'MuiAccordionSummary' })(AccordionSummary);
34 changes: 9 additions & 25 deletions packages/material-ui/src/AccordionSummary/AccordionSummary.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -64,33 +64,17 @@ describe('<AccordionSummary />', () => {
expect(expandIcon).toBeInaccessible();
});

it('focusing adds the `focused` class if focused visible', () => {
// TODO: Rename `focused` -> `focus-visible`
// `focused` is a global state which is applied on focus
// only here do we constrain it to focus-visible. THe name is also not consistent
// with :focus
const { getByRole } = render(<AccordionSummary />);
fireEvent.mouseDown(document.body); // pointer device
const button = getByRole('button');

fireEvent.keyDown(document.body, { key: 'Tab' }); // not actually focusing (yet)
button.focus();

expect(button).toHaveFocus();
expect(button).to.have.class(classes.focused);
});

it('blur should unset focused state', () => {
const { getByRole } = render(<AccordionSummary />);
fireEvent.mouseDown(document.body); // pointer device
fireEvent.keyDown(document.body, { key: 'Tab' }); // not actually focusing (yet)
const button = getByRole('button');
button.focus();
it('should fire onBlur when the button blurs', () => {
const handleBlur = spy();
const { getByRole } = render(<AccordionSummary onBlur={handleBlur} />);

button.blur();
act(() => {
const button = getByRole('button');
button.focus();
button.blur();
});

expect(button).not.toHaveFocus();
expect(button).not.to.have.class(classes.focused);
expect(handleBlur.callCount).to.equal(1);
});

it('should fire onClick callbacks', () => {
Expand Down

0 comments on commit e000cf3

Please sign in to comment.