Skip to content

Commit

Permalink
less ambitious changes
Browse files Browse the repository at this point in the history
  • Loading branch information
@oliviertassinari
oliviertassinari committed Nov 25, 2020
1 parent 22d53e2 commit 4beecd0
Show file tree
Hide file tree
Showing 4 changed files with 157 additions and 111 deletions.
41 changes: 24 additions & 17 deletions packages/material-ui/src/Unstable_TrapFocus/Unstable_TrapFocus.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,25 +43,31 @@ function getTabIndex(node) {
return node.tabIndex;
}

function isRadioTabble(node) {
function isNonTabbableRadio(node) {
if (node.tagName !== 'INPUT' || node.type !== 'radio') {
return false;
}

if (!node.name) {
return true;
return false;
}

let input = node.ownerDocument.querySelector(`input[type="radio"][name="${node.name}"]:checked`);
const getRadio = (selector) => node.ownerDocument.querySelector(`input[type="radio"]${selector}`);

let roving = getRadio(`[name="${node.name}"]:checked`);

if (!input) {
input = node.ownerDocument.querySelector(`input[type="radio"][name="${node.name}"]`);
if (!roving) {
roving = getRadio(`[name="${node.name}"]`);
}

return input === node;
return roving !== node;
}

function isFocusable(node) {
function isNodeMatchingSelectorFocusable(node) {
if (
node.disabled ||
(node.tagName === 'INPUT' && node.type === 'hidden') ||
(node.tagName === 'INPUT' && node.type === 'radio' && !isRadioTabble(node))
isNonTabbableRadio(node)
) {
return false;
}
Expand All @@ -75,7 +81,7 @@ export function defaultGetTabbable(root) {
Array.from(root.querySelectorAll(candidatesSelector)).forEach((node, i) => {
const nodeTabIndex = getTabIndex(node);

if (nodeTabIndex === -1 || !isFocusable(node)) {
if (nodeTabIndex === -1 || !isNodeMatchingSelectorFocusable(node)) {
return;
}

Expand Down Expand Up @@ -174,12 +180,7 @@ function Unstable_TrapFocus(props) {
}

if (activated.current) {
const tabbable = getTabbable(rootRef.current);
if (tabbable.length > 0) {
tabbable[0].focus();
} else {
rootRef.current.focus();
}
rootRef.current.focus();
}
}

Expand Down Expand Up @@ -244,7 +245,13 @@ function Unstable_TrapFocus(props) {
return;
}

const tabbable = getTabbable(rootRef.current);
let tabbable = [];
if (
doc.activeElement === sentinelStart.current ||
doc.activeElement === sentinelEnd.current
) {
tabbable = getTabbable(rootRef.current);
}

if (tabbable.length > 0) {
const isShiftTab = Boolean(
Expand Down Expand Up @@ -303,7 +310,7 @@ function Unstable_TrapFocus(props) {
doc.removeEventListener('focusin', contain);
doc.removeEventListener('keydown', loopFocus, true);
};
}, [disableAutoFocus, disableEnforceFocus, disableRestoreFocus, isEnabled, open]);
}, [disableAutoFocus, disableEnforceFocus, disableRestoreFocus, isEnabled, open, getTabbable]);

const onFocus = (event) => {
if (!activated.current) {
Expand Down
133 changes: 48 additions & 85 deletions packages/material-ui/src/Unstable_TrapFocus/Unstable_TrapFocus.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,10 +26,10 @@ describe('<TrapFocus />', () => {
document.body.removeChild(initialFocus);
});

it('should return focus to the children', () => {
it('should return focus to the root', () => {
const { getByTestId } = render(
<TrapFocus {...defaultProps} open>
<div tabIndex={-1} data-testid="modal">
<div tabIndex={-1} data-testid="root">
<input autoFocus data-testid="auto-focus" />
</div>
</TrapFocus>,
Expand All @@ -38,7 +38,7 @@ describe('<TrapFocus />', () => {
expect(getByTestId('auto-focus')).toHaveFocus();

initialFocus.focus();
expect(getByTestId('auto-focus')).toHaveFocus();
expect(getByTestId('root')).toHaveFocus();
});

it('should not return focus to the children when disableEnforceFocus is true', () => {
Expand Down Expand Up @@ -71,7 +71,7 @@ describe('<TrapFocus />', () => {
expect(getByTestId('auto-focus')).toHaveFocus();
});

it('should warn if the modal content is not focusable', () => {
it('should warn if the root content is not focusable', () => {
const UnfocusableDialog = React.forwardRef((_, ref) => <div ref={ref} />);

expect(() => {
Expand All @@ -96,15 +96,17 @@ describe('<TrapFocus />', () => {
it('should loop the tab key', () => {
render(
<TrapFocus {...defaultProps} open>
<div tabIndex={-1} data-testid="modal">
<div tabIndex={-1} data-testid="root">
<div>Title</div>
<button type="button">x</button>
<button type="button">cancel</button>
<button type="button">ok</button>
</div>
</TrapFocus>,
);
expect(screen.getByTestId('root')).toHaveFocus();

userEvent.tab();
expect(screen.getByText('x')).toHaveFocus();
userEvent.tab();
expect(screen.getByText('cancel')).toHaveFocus();
Expand All @@ -114,15 +116,16 @@ describe('<TrapFocus />', () => {
expect(screen.getByText('x')).toHaveFocus();

initialFocus.focus();
expect(screen.getByText('x')).toHaveFocus();
expect(screen.getByTestId('root')).toHaveFocus();
screen.getByText('x').focus();
userEvent.tab({ shift: true });
expect(screen.getByText('ok')).toHaveFocus();
});

it('should focus on first focus element after last has received a tab click', () => {
render(
<TrapFocus {...defaultProps} open>
<div tabIndex={-1} data-testid="modal">
<div tabIndex={-1} data-testid="root">
<div>Title</div>
<button type="button">x</button>
<button type="button">cancel</button>
Expand All @@ -131,67 +134,23 @@ describe('<TrapFocus />', () => {
</TrapFocus>,
);

userEvent.tab();
expect(screen.getByText('x')).toHaveFocus();
userEvent.tab();
expect(screen.getByText('cancel')).toHaveFocus();
userEvent.tab();
expect(screen.getByText('ok')).toHaveFocus();
userEvent.tab();
expect(screen.getByText('x')).toHaveFocus();
});

it('should focus rootRef if no tabbable children are rendered', () => {
render(
<TrapFocus {...defaultProps} open>
<div tabIndex={-1} data-testid="modal">
<div tabIndex={-1} data-testid="root">
<div>Title</div>
</div>
</TrapFocus>,
);
expect(screen.getByTestId('modal')).toHaveFocus();
});

it('should focus checked radio button if present in radio group', () => {
const Test = () => {
const [value, setValue] = React.useState('two');
const onChange = (e) => setValue(e.target.value);
return (
<TrapFocus {...defaultProps} open>
<div tabIndex={-1} data-testid="modal">
<div>Title</div>
<input
aria-label="one"
checked={value === 'one'}
id="one"
name="one"
onChange={onChange}
type="radio"
value="one"
/>
<input
aria-label="two"
checked={value === 'two'}
id="two"
name="two"
onChange={onChange}
type="radio"
value="two"
/>
<input
aria-label="three"
checked={value === 'three'}
id="three"
name="three"
onChange={onChange}
type="radio"
value="three"
/>
</div>
</TrapFocus>
);
};
render(<Test />);
userEvent.tab();
expect(screen.getByLabelText('two')).toHaveFocus();
expect(screen.getByTestId('root')).toHaveFocus();
});

it('does not steal focus from a portaled element if any prop but open changes', () => {
Expand Down Expand Up @@ -271,7 +230,7 @@ describe('<TrapFocus />', () => {
return (
<div onBlur={() => eventLog.push('blur')}>
<TrapFocus getDoc={() => document} isEnabled={() => true} open {...props}>
<div data-testid="focus-root" tabIndex={-1}>
<div data-testid="root" tabIndex={-1}>
<input data-testid="focus-input" />
</div>
</TrapFocus>
Expand All @@ -283,7 +242,7 @@ describe('<TrapFocus />', () => {
// same behavior, just referential equality changes
setProps({ isEnabled: () => true });

expect(screen.getByTestId('focus-input')).toHaveFocus();
expect(screen.getByTestId('root')).toHaveFocus();
expect(eventLog).to.deep.equal([]);
});

Expand Down Expand Up @@ -314,7 +273,7 @@ describe('<TrapFocus />', () => {
disableRestoreFocus
{...props}
>
<div data-testid="focus-root" tabIndex={-1}>
<div data-testid="root" tabIndex={-1}>
<input data-testid="focus-input" />
</div>
</TrapFocus>
Expand All @@ -325,7 +284,7 @@ describe('<TrapFocus />', () => {
setProps({ open: false, disableRestoreFocus: false });

// undesired: should be expect(initialFocus).toHaveFocus();
expect(screen.getByTestId('focus-input')).toHaveFocus();
expect(screen.getByTestId('root')).toHaveFocus();
});

it('undesired: setting `disableRestoreFocus` to false before closing has no effect', () => {
Expand All @@ -338,7 +297,7 @@ describe('<TrapFocus />', () => {
disableRestoreFocus
{...props}
>
<div data-testid="focus-root" tabIndex={-1}>
<div data-testid="root" tabIndex={-1}>
<input data-testid="focus-input" />
</div>
</TrapFocus>
Expand All @@ -350,7 +309,7 @@ describe('<TrapFocus />', () => {
setProps({ open: false });

// undesired: should be expect(initialFocus).toHaveFocus();
expect(screen.getByTestId('focus-input')).toHaveFocus();
expect(screen.getByTestId('root')).toHaveFocus();
});

describe('interval', () => {
Expand All @@ -368,23 +327,27 @@ describe('<TrapFocus />', () => {
function WithRemovableElement({ hideButton = false }) {
return (
<TrapFocus {...defaultProps} open>
<div tabIndex={-1} role="dialog">
{!hideButton && <button type="button">I am going to disappear</button>}
<div tabIndex={-1} data-testid="root">
{!hideButton && (
<button type="button" data-testid="hide-button">
I am going to disappear
</button>
)}
</div>
</TrapFocus>
);
}

const { getByRole, setProps } = render(<WithRemovableElement />);
const dialog = getByRole('dialog');
const toggleButton = getByRole('button', { name: 'I am going to disappear' });
const { setProps } = render(<WithRemovableElement />);

expect(toggleButton).toHaveFocus();
expect(screen.getByTestId('root')).toHaveFocus();
screen.getByTestId('hide-button').focus();
expect(screen.getByTestId('hide-button')).toHaveFocus();

setProps({ hideButton: true });
expect(dialog).not.toHaveFocus();
expect(screen.getByTestId('root')).not.toHaveFocus();
clock.tick(500); // wait for the interval check to kick in.
expect(dialog).toHaveFocus();
expect(screen.getByTestId('root')).toHaveFocus();
});

describe('prop: disableAutoFocus', () => {
Expand All @@ -393,7 +356,7 @@ describe('<TrapFocus />', () => {
<div>
<input />
<TrapFocus {...defaultProps} open disableAutoFocus>
<div tabIndex={-1} data-testid="modal" />
<div tabIndex={-1} data-testid="root" />
</TrapFocus>
</div>,
);
Expand All @@ -406,56 +369,56 @@ describe('<TrapFocus />', () => {
});

it('should trap once the focus moves inside', () => {
const { getByRole } = render(
render(
<div>
<input />
<input data-testid="outside-input" />
<TrapFocus {...defaultProps} open disableAutoFocus>
<div tabIndex={-1} data-testid="modal">
<button data-testid="focus-input" />
<div tabIndex={-1} data-testid="root">
<button type="buton" data-testid="focus-input" />
</div>
</TrapFocus>
</div>,
);

expect(initialFocus).toHaveFocus();

userEvent.tab();
expect(getByRole('textbox')).toHaveFocus();
screen.getByTestId('outside-input').focus();
expect(screen.getByTestId('outside-input')).toHaveFocus();

// the trap activates
userEvent.tab();
expect(screen.getByTestId('focus-input')).toHaveFocus();

// the trap prevent to escape
getByRole('textbox').focus();
expect(screen.getByTestId('focus-input')).toHaveFocus();
screen.getByTestId('outside-input').focus();
expect(screen.getByTestId('root')).toHaveFocus();
});

it('should restore the focus', () => {
const Test = (props) => (
<div>
<input data-testid="outside-input" />
<TrapFocus {...defaultProps} open disableAutoFocus {...props}>
<div tabIndex={-1} data-testid="modal">
<div tabIndex={-1} data-testid="root">
<input data-testid="focus-input" />
</div>
</TrapFocus>
</div>
);

const { getByTestId, setProps } = render(<Test />);
const { setProps } = render(<Test />);

// set the expected focus restore location
getByTestId('outside-input').focus();
expect(getByTestId('outside-input')).toHaveFocus();
screen.getByTestId('outside-input').focus();
expect(screen.getByTestId('outside-input')).toHaveFocus();

// the trap activates
getByTestId('modal').focus();
expect(getByTestId('modal')).toHaveFocus();
screen.getByTestId('root').focus();
expect(screen.getByTestId('root')).toHaveFocus();

// restore the focus to the first element before triggering the trap
setProps({ open: false });
expect(getByTestId('outside-input')).toHaveFocus();
expect(screen.getByTestId('outside-input')).toHaveFocus();
});
});
});
Expand Down
Loading

0 comments on commit 4beecd0

Please sign in to comment.