feat(tour): add navigation.autoFocus to control where step focus lands - #27
Conversation
A tour panel is role="dialog" and focus moves into it when a step opens, so
the dialog is announced and Escape is reachable. Until now that focus always
landed on the panel element, which means Enter does nothing until the user
Tabs to a control - wrong for a read-and-advance tour, and not fixable from
outside the library.
navigation: { showControls: true, autoFocus: 'primary' }
'panel' - the panel itself (default, unchanged)
'primary' - Next, or Finish on the last step, so Enter advances
false - leave focus where it is
'primary' resolves through a data-tip-magic-primary attribute that the
content stamps on its own primary action, rather than the tooltip querying
for tour classes - the tooltip stays unaware of what a tour is. It falls back
to the panel when a step renders no primary action, and never to the close
button, where Enter would end the tour.
Settable per tour and per step, like the rest of TourNavigation. It lives
there rather than at tour level for two reasons: the focus move only happens
for steps that render as a dialog, which is exactly when navigation features
are present, and TourOptions.focus already means the backdrop - an adjacent
autoFocus would be two confusable names for unrelated jobs.
Fixes a prerequisite bug found while building it
------------------------------------------------
React re-applies dangerouslySetInnerHTML on every re-render even when the
string is byte-identical, rebuilding the whole subtree. Focusing anything
inside tour content was therefore unstable: the next re-render - a position
update, a visibility change - destroyed the focused node and dropped focus to
<body>. Reproduced in isolation, no library code involved.
The HTML content span is now memoised on its content string, so the subtree is
only rebuilt when the content really changes. Beyond making autoFocus work,
this stops the tour panel's markup being re-parsed on every tooltip re-render
and stops an <img> or autoplaying <video> inside a step being recreated, which
restarted playback.
Tests: default unchanged on first and later steps; 'primary' focuses Next then
Finish; reaches the button even when focus is already on the panel (the guard
that would have blocked it used containment, and Node.contains includes the
node itself); falls back to the panel and not close with no controls; false
leaves focus alone; restore on end works in all three modes; step-level
override; inert for non-dialog steps. Plus three for the memoisation.
Also: Storybook story with a live focus readout, Flows.mdx and README.
Code review pass over the branch. No behaviour change - 377 tests pass, including the nine covering the option and the three covering memoisation. Duplication removed - 'tip-magic-text' was an inline string in two places while CSS_CLASSES exists for exactly this. Added TOOLTIP_TEXT and used it; one of the two call sites predates this branch. - The primary-action attribute had two names: PRIMARY_ACTION_ATTRIBUTE in the core constants and TOUR_DATA_ATTRIBUTES.PRIMARY_ACTION re-exporting it. Dropped the alias, which also removes a core -> tour import. - TooltipAutoFocus and TourAutoFocus each spelled out the same union members and could drift. TourAutoFocus is now an alias. - The autoFocus tests repeated the same options and markup literals seven times, and the memoisation tests rebuilt mock state four times. Both now go through one local helper. Separation Focus target resolution moved out of the effect into utils/autoFocusTarget.ts, matching how tooltipStyles and groupCompatibility hold Tooltip's pure logic. It has 11 unit tests of its own, and expressing the two different guards - identity for 'primary', containment for 'panel' - as one returned value is what let the inline comments go. Edge case A marked primary element that cannot take focus - disabled, or hidden - made focus() a silent no-op, leaving focus outside the dialog: the exact failure the fallback exists to prevent. The effect now checks whether focus landed and falls back to the panel. Comments Removed the implementation comments. The gotchas they carried are in a new CLAUDE.md: dangerouslySetInnerHTML being re-applied every render, Node.contains including the node itself, DOMTokenList.add re-setting the class attribute, React rewriting class wholesale, the panel being a non-modal dialog, never marking Close as primary, step content being HTML, onStepChange firing before the panel renders, and the two things jsdom cannot answer. CLAUDE.md also records the layout, the constants and pure-util conventions, that DATA_ATTRIBUTES is dead, and the registry-gated release process. Docs Flows.mdx accessibility section now covers the dialog role, the deliberate absence of a focus trap, focus restore, and links to autoFocus.
galangel
left a comment
galangel
left a comment
There was a problem hiding this comment.
Approving. I checked the load-bearing claims out rather than taking them on trust, and they hold up. No must-fix, no bug found.
What I verified independently
The dangerouslySetInnerHTML claim is real on the React we ship against. React 19.2.3's setProp in react-dom-client does domElement.innerHTML = value.__html with no comparison against the previous html, and updateProperties reaches that branch on every render because {__html} is a fresh object each time. So the memoisation is not defensive coding, it is fixing a live subtree rebuild.
The memoisation tests are not vacuous. I removed the memo() wrapper and re-ran Tooltip.test.tsx: 2 of the 3 fail (keeps the same nodes…, keeps focus that is inside the content). They fail for the stated reason.
Enter on the focused Next button cannot double-advance. useTour binds only Escape globally — there is no Enter/Space keybinding to race the button's native activation. The click handler's e.detail !== 0 guard and the mousedown path stay disjoint, so autoFocus: 'primary' does not open a double-fire.
autoFocus cannot promote a plain tooltip into a dialog. hasNavigationFeatures is untouched and does not read the key; the "inert for a step that is not a dialog" test pins it.
The default path really is unchanged. resolveAutoFocusTarget bottoms out at panel.contains(activeElement) ? null : panel, which is the old guard verbatim.
'primary' will not steal focus back mid-step. This was my main worry, since the resolver deliberately takes focus from another control inside the panel. It is safe because the effect's deps cannot change during a step: floating-ui keeps isPositioned at true across autoUpdate recomputations, so scroll and resize do not re-run it. Focus only moves on open and on a genuine content change.
The disabled/unfocusable-primary fallback works. focus() no-ops, panel.contains(document.activeElement) stays false, panel catches it. Good catch adding that in the second commit — that was the one path where focus could have escaped the dialog entirely.
npm run validate is clean locally (377 tests, 20 files), all three CI jobs green. I also spot-checked the CLAUDE.md gotchas: DATA_ATTRIBUTES is genuinely unreferenced, there is no aria-modal anywhere, the validate script matches, and the DOMTokenList.add claim is correct per spec (add() always runs the update steps, so the attribute is re-set even for a token already present). Bumping the version in the feature PR matches precedent — #15 did 1.1.1 → 1.2.0 the same way.
Non-blocking notes
-
PRIMARY_ACTION_ATTRIBUTEis not exported fromsrc/index.ts.TooltipShowOptions.autoFocusis public API for any programmaticrole: 'dialog'tooltip, and the design note is explicit that the attribute is the contract between content and tooltip — but a consumer outside a tour has to hardcodedata-tip-magic-primary. Exporting the constant next toescapeHtmlwould close the loop, and it matches the convention CLAUDE.md just wrote down. -
Two doors to the same setting in
useTour. The forced block setsautoFocus: mergedNav.autoFocusafter spreadingtooltipOptionsandstep.tooltipOptions, andmergedNav.autoFocusalways resolves throughDEFAULT_NAVIGATION— sotooltipOptions: { autoFocus: 'primary' }is silently overridden by the navigation default. Structurally it is the same asrole/html/interactive, but those are genuinely non-negotiable whereas this one is user-configurable through a second door. A sentence sayingnavigation.autoFocusis the only knob for tour steps would save someone the debug. -
Sidebar nesting.
title: 'The Tourtip/autoFocus'makes the existing'The Tourtip'both a leaf and a group. Storybook builds fine, it just reads a little oddly next toTooltip/Flow & Tours. -
The
TourAutoFocus = TooltipAutoFocusalias dropped the "so Enter advances the tour" wording from the per-member docs.TourNavigation.autoFocusstill carries it, so nothing is lost in practice — only the hover text on the alias itself got thinner. Deduping the union was the right call.
Agreed on both things you left open: transitionBehavior: 'move' timing and a screen-reader pass need a real browser, and not claiming them from a passing jsdom test is the right instinct. The focus-ring question in the report is almost certainly host CSS or the UA :focus-visible heuristic, as you say — worth telling them either way, and it does not change the value of this option.
2 participants
Adds
navigation.autoFocus, so a consumer can put focus on the step's primary action instead of the panel. Requested after a tour in a real app: on a read-and-advance tour, focus on the panel means Enter does nothing until the user Tabs.'panel'(default, unchanged)'primary'falseSettable per tour and per step, like the rest of
TourNavigation.Why it needs to be in the library
Doing it from outside requires selecting on
.tip-magic-tour-btn-next— a styling class used as a behavioural handle — and winning a race the consumer cannot win.onStepChangefires from insidestart()before the panel exists, so there is nothing to focus on the first step. Verified:Inside the library there is no race at all — the focus effect is keyed on the content and the button is already in the DOM when it runs, so no
requestAnimationFrameor retry is involved.A prerequisite bug this had to fix first
React re-applies
dangerouslySetInnerHTMLon every re-render, even when the string is byte-identical. The subtree is rebuilt, so focusing anything inside tour content was unstable: the next re-render — a position update, a visibility change — destroyed the focused node and dropped focus to<body>.Reproduced with no library code involved:
The HTML content span is now memoised on its content string, so the subtree is only rebuilt when the content actually changes. That was needed to make
autoFocuswork at all, and it independently fixes two things that were already happening onmain:<img>(including a GIF) or autoplaying<video>inside a step was recreated on every re-render, restarting playbackThree tests cover it: nodes and media preserved across a no-op re-render, focus inside the content preserved, and the subtree still rebuilt when the content really changes.
Design notes
'primary'is resolved through an attribute, not a class.buildNavHtmlstampsdata-tip-magic-primaryon whichever button is the step's primary action, and the tooltip focuses[data-tip-magic-primary]. The tooltip therefore stays unaware of what a tour is — it does not query tour classes ordata-tour-action— and'primary'means something coherent for any programmatic tooltip, not just a tour.The fallback never picks Close. With
showControlsoff,showClosestill defaults to true, so the panel contains exactly one button — Close. Anything reaching for "the first button" would put Enter on end the tour. The chain is strictly: marked primary → panel.The guard had to change, and this is where the default could have broken. The existing effect skipped when
panel.contains(document.activeElement).Node.containsincludes the node itself, so once focus is on the panel — exactly where the default puts it — a naive'primary'implementation would never reach the button. It would appear to work on a fresh tour and silently stop after that.'primary'now guards on identity;'panel'keeps the containment check verbatim, so the default path is unchanged.Why
TourNavigationand notTourOptions. The focus move only happens for steps that render asrole="dialog", which is exactly whenhasNavigationFeaturesis true — so the option is only meaningful alongside navigation features. It also avoids sitting next toTourOptions.focus, which means the backdrop; two adjacent options namedfocusandautoFocusdoing unrelated jobs would be a trap.hasNavigationFeaturesis untouched, so adding the key cannot promote a plain tooltip into a dialog.Verification
npm run validateclean — typecheck, lint, format, 366 tests (354 onmain). Library and Storybook both build.Nine tests for the option: default unchanged on the first and later steps;
'primary'focuses Next then Finish; reaches the button even when focus is already on the panel; falls back to the panel and not Close with no controls;falseleaves focus untouched; restore-on-end works in all three modes; step-level override beats tour-level; inert for a non-dialog step.Not covered here
Two things I could not settle in jsdom and have not claimed:
transitionBehavior: 'move'. Both modes land focus in the harness, but jsdom does not run transitions, so that says nothing about the real timing. Worth a look in a browser.aria-labelledbyare unchanged and focus still moves inside the dialog, so it should hold, but I have not put it in front of a reader.There is also a question worth raising separately: the motivating report described a focus ring on the panel on every step. The library ships no focus styling for the tooltip at all — no
outlineor:focusrule anywhere — so that ring is either the UA:focus-visiblerule, which for programmatic focus only applies when the last interaction was a keyboard one, or the host app's own global:focusCSS.autoFocus: 'primary'is worth having either way for the Enter behaviour, but if the ring shows up under mouse-only operation it is app CSS, which is useful for them to know.