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

reset active element/focus order state on ui close #158

Merged
merged 2 commits into from
Feb 26, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions src/components/App.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,11 @@ export function App({
initialViewState,
dismissedViewState,
eventTarget,
savedActiveElement:
document.activeElement instanceof HTMLElement &&
document.activeElement !== document.body
? document.activeElement
: null,
});

// Language setup
Expand Down
21 changes: 20 additions & 1 deletion src/hooks/useViewState.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,16 @@ export function useViewState({
initialViewState,
dismissedViewState,
eventTarget,
savedActiveElement,
}: {
/** Which state this consent manager should go to when opened */
initialViewState: InitialViewState;
/** Which state this consent manager should go to when closed */
dismissedViewState: DismissedViewState;
/** The event target on the `transcend` API, where we will dispatch view state change events */
eventTarget: EventTarget;
/** Element previously focused before our ui modal was opened */
savedActiveElement: HTMLElement | null;
}): {
/** The current view state */
viewState: ViewState;
Expand Down Expand Up @@ -118,14 +121,30 @@ export function useViewState({
break;

// Request to close the modal, to the closed view state (which depends on whether customer wants to display the collapse view or hide it)
case 'close':
case 'close': {
setState({
current: dismissedViewState,
previous: state.current,
auth,
firstSelectedViewState: null,
});
/* If the user previously had something focused before the modal popped up, then focus it.
* Otherwise, we have to do this really janky "reset" behavior with the temporary element
* because Chrome specifically has some weird internal state around focus order that is
* very difficult to interact with. We create an element with maximum focus priority and
* focus it so that when we delete it the user will be at the start of the focus order
* just like if they had freshly loaded the page. */
Comment on lines +131 to +136
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice!

if (savedActiveElement !== null) {
savedActiveElement.focus();
} else {
const tempInteractiveEl = document.createElement('span');
tempInteractiveEl.setAttribute('tabindex', '1');
document.body.prepend(tempInteractiveEl);
tempInteractiveEl.focus();
tempInteractiveEl.remove();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: maybe a note to describe why this is needed?

also, i'm sorta confused on the last two steps? the temp element is being focused on, so that brings focus to the new temp element for the first interactive/tab indexed element on the page? why then remove? is it so the focus can then be shifted around by the user?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh youre so right about the comment, i should have added one originally.

basically this function just "resets" focus order state, so that the next time the user presses tab it will go to the first interactive element on the page like it would on page load. we remove it because once its reset the focus state it no longer serves a purpose, and bc if we didnt remove it then we would leave a useless html element lying around in the document body which is super messy

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ooooh thanks for explaining the removal!

}
break;
}

// Request to go to a specific view state, e.g. the language options page
default:
Expand Down
Loading