Page MenuHomePhabricator

Dialog: Ensure focus is on triggering element or previous section heading after closing
Closed, ResolvedPublic2 Estimated Story Points

Description

Background

From DST's evaluating engagement with our partner AFB, we've been provided with a number of answers and tips to possible shortcomings/improvements on several of our components

In the case of the Dialog, we were very uncertain about the focus being in the right place after the dialog is opened working in a predictable way? Putting dialogs before end body tag) .

The feedback received was:

The focus is appropriately set when the dialog is opened. However, upon dismissing or closing the dialog, the focus is shifted to the heading level 1. It should instead be maintained on the element that triggered the dialog or the previous section heading where the dialog was opened.

Goal

Put focus on triggering element or previous section heading after closing

Acceptance criteria

  • Implement focus handler on Dialog exit

Event Timeline

Volker_E triaged this task as Medium priority.May 10 2024, 11:03 PM

So, the code to do this in the Dialog component is pretty trivial:

// create a new ref to stash the focus value when dialog opens
const previouslyFocused = ref<Element|null>( null );

When the Dialog's onDialogOpen method is called, add the following lines:

previouslyFocused.value = document.activeElement;

And when the Dialog's onDialogClose method is called, restore focus via the following (the extensive conditional is here to keep typescript happy)

if (
    previouslyFocused.value &&
    'focus' in previouslyFocused.value &&
    typeof previouslyFocused.value.focus === 'function'
) {
    previouslyFocused.value.focus();
    previouslyFocused.value = null;
}

This all works fine. However, in cases where a Codex Button component is the trigger that opens the dialog (which is probably the most common use-case), the button styles start to break: you don't see a blue outline as expected, and the button remains stuck in the active styling state:

{F56298138}

This is a pre-existing problem in Codex, but fixing this Dialog issue has made it obvious. It looks like the setActive() code that was added to the button a while back does not handle this situation very well.

Change #1052820 had a related patch set uploaded (by Eric Gardner; author: Eric Gardner):

[design/codex@main] Dialog: restore focus to previously-focused element on close

https://gerrit.wikimedia.org/r/1052820

Change #1052820 merged by jenkins-bot:

[design/codex@main] Dialog: restore focus to previously-focused element on close

https://gerrit.wikimedia.org/r/1052820

Change #1056999 had a related patch set uploaded (by LWatson; author: LWatson):

[mediawiki/core@master] Update Codex from v1.9.0 to v1.10.0

https://gerrit.wikimedia.org/r/1056999