Skip to content

Commit

Permalink
[core] Reduce calls to actions prop
Browse files Browse the repository at this point in the history
imperative handles were created on every render without any need.
We can use deps and eslint to safely reduce those to a bare
minimum
  • Loading branch information
eps1lon committed Apr 11, 2019
1 parent 1d6425d commit 397ccee
Show file tree
Hide file tree
Showing 2 changed files with 49 additions and 41 deletions.
64 changes: 34 additions & 30 deletions packages/material-ui/src/MenuList/MenuList.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,37 +37,41 @@ const MenuList = React.forwardRef(function MenuList(props, ref) {
const listRef = React.useRef();
const selectedItemRef = React.useRef();

React.useImperativeHandle(actions, () => ({
focus: () => {
if (selectedItemRef.current) {
selectedItemRef.current.focus();
return;
}
React.useImperativeHandle(
actions,
() => ({
focus: () => {
if (selectedItemRef.current) {
selectedItemRef.current.focus();
return;
}

if (listRef.current && listRef.current.firstChild) {
listRef.current.firstChild.focus();
}
},
getContentAnchorEl: () => {
if (selectedItemRef.current) {
return selectedItemRef.current;
}
return listRef.current.firstChild;
},
adjustStyleForScrollbar: (containerElement, theme) => {
// Let's ignore that piece of logic if users are already overriding the width
// of the menu.
const noExplicitWidth = !listRef.current.style.width;
if (containerElement.clientHeight < listRef.current.clientHeight && noExplicitWidth) {
const scrollbarSize = `${getScrollbarSize(true)}px`;
listRef.current.style[
theme.direction === 'rtl' ? 'paddingLeft' : 'paddingRight'
] = scrollbarSize;
listRef.current.style.width = `calc(100% + ${scrollbarSize})`;
}
return listRef.current;
},
}));
if (listRef.current && listRef.current.firstChild) {
listRef.current.firstChild.focus();
}
},
getContentAnchorEl: () => {
if (selectedItemRef.current) {
return selectedItemRef.current;
}
return listRef.current.firstChild;
},
adjustStyleForScrollbar: (containerElement, theme) => {
// Let's ignore that piece of logic if users are already overriding the width
// of the menu.
const noExplicitWidth = !listRef.current.style.width;
if (containerElement.clientHeight < listRef.current.clientHeight && noExplicitWidth) {
const scrollbarSize = `${getScrollbarSize(true)}px`;
listRef.current.style[
theme.direction === 'rtl' ? 'paddingLeft' : 'paddingRight'
] = scrollbarSize;
listRef.current.style.width = `calc(100% + ${scrollbarSize})`;
}
return listRef.current;
},
}),
[],
);

React.useEffect(() => {
resetTabIndex(listRef.current, selectedItemRef.current, setCurrentTabIndex);
Expand Down
26 changes: 15 additions & 11 deletions packages/material-ui/src/RadioGroup/RadioGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,23 @@ const RadioGroup = React.forwardRef(function RadioGroup(props, ref) {
return null;
});

React.useImperativeHandle(actions, () => ({
focus: () => {
let input = rootRef.current.querySelector('input:not(:disabled):checked');
React.useImperativeHandle(
actions,
() => ({
focus: () => {
let input = rootRef.current.querySelector('input:not(:disabled):checked');

if (!input) {
input = rootRef.current.querySelector('input:not(:disabled)');
}
if (!input) {
input = rootRef.current.querySelector('input:not(:disabled)');
}

if (input) {
input.focus();
}
},
}));
if (input) {
input.focus();
}
},
}),
[],
);

React.useEffect(() => {
warning(
Expand Down

0 comments on commit 397ccee

Please sign in to comment.