Repository navigation
Conversation
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
It refactors core focus handling across disclosure, collapse, modal, tab, and navigation components with cross-component side effects and no JS unit tests, plus a flagged preventFocus behavior divergence, so it warrants human verification.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
This PR fixes focus management when disclosures (accordions/collapses, modals, tabs) are closed, addressing issue #1505 where opening an accordion that is initially expanded (aria-expanded="true") and then closing it could lose focus or send it to the header logo — and could throw when the header JS was not loaded. It centralizes the "focus the activated button + memorize the outside trigger" logic in DisclosureButton, and lets each component decide where focus should return on close.
Changes:
- Move explicit button focus + trigger memorization (
retainFocus) intoDisclosureButton.handleClick, and add a defaultfocus()that focuses the primary button; remove the now-redundantretainFocusfrom basediscloseand the redundanthandleClick/focus inTabButton. - Rework
Collapse.concealto return focus to the main button when focus was inside the hidden content, or to the memorized trigger/logo when there is no active focus. - Make modals memorize/restore the pre-open focus (their internal close button no longer overwrites the target), and guard
FocusManager.focusOnLogowhen the header component is not loaded; fix a doc typo infieldset.ejs.
| File | Description |
|---|---|
| src/dsfr/core/script/disclosure/disclosure.js | Adds _focusIndex default, removes retainFocus from disclose, and makes focus() target the primary button. |
| src/dsfr/core/script/disclosure/disclosure-button.js | handleClick now focuses the button, memorizes external triggers, then toggles. |
| src/dsfr/core/script/collapse/collapse.js | conceal restores focus based on where focus currently is; potential preventFocus default divergence flagged. |
| src/dsfr/core/script/api/modules/register/focus-manager.js | Guards focusOnLogo when api.header is unavailable. |
| src/dsfr/component/tab/script/tab/tab-button.js | Removes redundant handleClick override now handled by the base class. |
| src/dsfr/component/modal/script/modal/modal.js | Memorizes focus on disclose and overrides focus() to restore it. |
| src/dsfr/component/form/template/ejs/fieldset/fieldset.ejs | Fixes doc typo massages → messages. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| const activeElement = document.activeElement; | ||
| const shouldFocus = this.node.contains(activeElement); | ||
| const hasNoFocus = !activeElement || activeElement === document.body || activeElement === document.documentElement; | ||
| if (!super.conceal(withhold, true) || preventFocus === true) return; |
#1505
Corrige la gestion du focus à la fermeture des disclosures, notamment lorsqu’un accordéon est ouvert au chargement. Sa fermeture pouvait renvoyer le focus au logo et provoquer une erreur si le JavaScript du header n’était pas chargé.