Skip to content

Commit

Permalink
[@mantine/hooks] use-focus-trap: Fix incorrect aria-hidden handling (#…
Browse files Browse the repository at this point in the history
…2735)

* [@mantine/hooks] use-focus-trap: Fix aria-hidden="true" not being removed

* [@mantine/hooks] use-focus-trap: Fix aria-hidden not being applied multiple times

* [@mantine/core] FocusTrap: Add test for aria-hidden
  • Loading branch information
Keilo75 committed Oct 22, 2022
1 parent 36b9504 commit 2599b47
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 7 deletions.
23 changes: 23 additions & 0 deletions src/mantine-core/src/FocusTrap/FocusTrap.test.tsx
Expand Up @@ -51,6 +51,29 @@ describe('@mantine/core/FocusTrap', () => {
expect(document.body).toHaveFocus();
});

it('manages aria-hidden attributes', () => {
const adjacentDiv = document.createElement('div');
adjacentDiv.setAttribute('data-testid', 'adjacent');
document.body.appendChild(adjacentDiv);

const { rerender } = render(
<FocusTrap active>
<div />
</FocusTrap>
);

const adjacent = screen.getByTestId('adjacent');
expect(adjacent).toHaveAttribute('aria-hidden', 'true');

rerender(
<FocusTrap active={false}>
<div />
</FocusTrap>
);

expect(adjacent).not.toHaveAttribute('aria-hidden');
});

it('has correct displayName', () => {
expect(FocusTrap.displayName).toEqual('@mantine/core/FocusTrap');
});
Expand Down
17 changes: 10 additions & 7 deletions src/mantine-hooks/src/use-focus-trap/use-focus-trap.ts
Expand Up @@ -13,18 +13,17 @@ export function useFocusTrap(active = true): (instance: HTMLElement | null) => v
return;
}

if (node === ref.current || node === null) {
if (node === null) {
return;
}

if (restoreAria.current) {
restoreAria.current();
restoreAria.current = createAriaHider(node);
if (ref.current === node) {
return;
}

if (node) {
const processNode = (_node: HTMLElement) => {
restoreAria.current = createAriaHider(_node);

const processNode = () => {
let focusElement: HTMLElement = node.querySelector('[data-autofocus]');

if (!focusElement) {
Expand All @@ -47,7 +46,7 @@ export function useFocusTrap(active = true): (instance: HTMLElement | null) => v
// Delay processing the HTML node by a frame. This ensures focus is assigned correctly.
setTimeout(() => {
if (node.ownerDocument) {
processNode(node);
processNode();
} else if (process.env.NODE_ENV === 'development') {
// eslint-disable-next-line no-console
console.warn('[@mantine/hooks/use-focus-trap] Ref node is not part of the dom', node);
Expand Down Expand Up @@ -76,6 +75,10 @@ export function useFocusTrap(active = true): (instance: HTMLElement | null) => v
document.addEventListener('keydown', handleKeyDown);
return () => {
document.removeEventListener('keydown', handleKeyDown);

if (restoreAria.current) {
restoreAria.current();
}
};
}, [active]);

Expand Down

0 comments on commit 2599b47

Please sign in to comment.