Skip to content

notifyPushView.closePush restores focus when push never had focus #869

Description

@swashbuck

Summary

notifyPushView.closePush() calls a11y.gotoPreviousActiveElement() unconditionally on every push close. Push notifications never grab focus on open (non-modal <dialog>, aria-modal=\"false\"). For pushes where the user did not tab into the push before it auto-closes, this fires a focus restore that has no contract to fulfil and can land focus in the wrong place.

Repro

  1. Use a course with adapt-contrib-trickle configured with _styleAfterClick: \"hidden\" (default) on a block. Add a next block with a heavy image so the load wait is observable.
  2. Add the additionalContentLoaded message so trickle calls notify.read (a11y-push) - see Update: Alert screen reader users when content has been added (fixes #232) adapt-contrib-trickle#233.
  3. Click the trickle Continue button via keyboard / screen reader.
  4. Trickle hides the clicked button container (u-display-none). Focus orphans to body.
  5. ~200ms later the a11y-push dialog opens and the screen reader reads "Loading".
  6. ~350ms later the push auto-closes. closePush calls gotoPreviousActiveElement. The previous active element was the trickle button which is now non-readable. focusFirst walks the DOM forward and lands on the first focusable element in Adapt.navigation (e.g. a navbar button).
  7. Trickle's own controller.scroll -> focusFirst(\$('.next-block')) either has not run yet (next block still loading) or runs after and is overridden.

Net: focus jumps to the navbar mid-flow.

Root cause

notifyPushView.closePush():

async closePush(event) {
  ...
  this.\$el[0].open = false;
  a11y.gotoPreviousActiveElement();
  ...
}

gotoPreviousActiveElement is correct for popups (modal, grab focus on open, restore on close). Push notifications are different:

  • Non-modal <dialog> opened via dialog.open = true (not showModal())
  • aria-modal=\"false\"
  • Never grab focus on open
  • May or may not receive focus during their lifetime (user can tab into the close button, click the body, etc.)

When the user never tabbed into the push, there is no focus contract to restore on close. Firing the restore can override correct focus management elsewhere (in trickle's case, the focusFirst that should land on the next block).

Proposed fix

Only restore focus on close if focus was actually inside the push at close time. Pattern already used in the same file for the Esc key handler (onKeyDown -> isFocusInPopup):

async closePush(event) {
  ...
  this.\$el[0].open = false;
  const isFocusInPush = Boolean(\$(document.activeElement).closest(this.\$el).length);
  if (isFocusInPush) {
    a11y.gotoPreviousActiveElement();
  }
  ...
}

Behaviour matrix:

Scenario Focus in push at close? Current With fix
a11y-push (notify.read) auto-close, user never tabbed in no gotoPreviousActiveElement (may fall back to navbar) no-op, focus stays where it is
Regular push auto-close, user never tabbed in no same as above no-op, focus stays where it is
User clicks the close button yes restore restore
User presses Esc while focus inside push yes (Esc handler already guards on isFocusInPopup) restore restore
User clicks the push body (_openNotifyOnClick) depends - click handler opens a separate popup which grabs focus navbar fallback popup handles its own focus contract

Net: behaviour only changes for the auto-close, user-not-focused-in path - which is the path that has no business shifting focus.

Scope

  • One-file change to js/views/notifyPushView.js
  • Modal popup behaviour (notifyPopupView) is correct as-is and not touched
  • No template, schema, or CSS changes

Related


Posted via collaboration with Claude Code

Metadata

Metadata

Assignees

Labels

Type

No fields configured for Bug.

Projects

Status
Needs Reviewing

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions