Make the rich text editor toolbars a single tab stop with arrow-key navigation by rtibblesbot · Pull Request #6108 · learningequality/studio

rtibblesbot · 2026-08-26T20:19:23Z

Summary

A useRovingTabIndex composable keeps exactly one toolbar control at tabindex="0" and moves focus with Left/Right, wrapping at both ends and reversed in RTL. Unavailable controls are marked aria-disabled rather than disabled, so they keep their place in the arrow order. Wired into all four role="toolbar" elements in the editor: EditorToolbar, MobileTopBar, MobileFormattingBar and LinkBubbleMenu.

References

Fixes #6104. Pattern: WAI-ARIA APG toolbar.

Reviewer guidance

Open a channel, edit an exercise, open the Questions tab, click a question, then the pencil on the Question field.
Right from any control steps through the toolbar and wraps from the last to the first.
Left on the first control lands on the minimize button.
Enter and Space each open the "Normal" text-format menu.
Left/Right step past that trigger while its menu is closed.
Tab out and Shift+Tab back — focus returns to the control you left.
Undo and Redo keep their place in the arrow order while greyed out, and do nothing when activated.
Narrow the window to the mobile layout — the top bar navigates the same way.
On a touch device, focus the text, then Shift+Tab — focus lands on the floating formatting bar, which is one tab stop and arrows the same way.
In the mobile layout, the + button opens the insert menu with Enter, arrows through its items and returns focus to + on close.

Toolbar with roving focus	Arrow navigation and wrapping

MobileFormattingBar also needed a mount fix: it rendered only while the editor content held focus, so Tab blurred the content and unmounted the bar before focus could land in it. It now renders while focus is anywhere in the editor container, clears its scroll-into-view timeout on unmount, gives its toggle and format-size buttons a focus-visible outline, and marks its scroll container tabindex="-1" so Chrome stops giving that container its own tab stop.

The mobile top bar's hand-rolled insert dropdown is now a KDropdownMenu: the old one had no menu semantics, keyboard navigation or focus return.

ClickableRegion lost the z-index on its content wrapper: the stacking context it opened painted the formatting bar and the link, image and math popovers behind the next answer option.

AI usage

Used Claude Code to write the composable and its tests test-first against a pre-agreed plan, and to drive the toolbar in a browser for the capture above. Verified with the full Jest suite, pre-commit, an axe-core audit, and keyboard QA in Chromium.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

How was this generated?

Ran pre-flight CI checks (lint, format, tests) and verified all pass
Rebased onto the target branch and resolved any conflicts
Reorganized commit history into clean, logical commits
Audited the diff to ensure only issue-relevant files are changed
Built PR body from the repository's PR template with evidence blocks

	🟡 Waiting for feedback Last updated: 2026-09-01 13:15 UTC

AlexVelezLl

Hey @rtibblesbot, on mobile, the mobile toolbar is not reachable via tab, could you fix it please, and make that the corresponding tab position on the RTE toolbar goes to the mobile toolbar instead?

rtibblesbot · 2026-08-27T14:54:20Z

@AlexVelezLl fixed in 1c54e96.

The floating formatting bar rendered only while the editor content held focus (v-if="isFocused"), so Tab blurred the content and unmounted the bar before focus could land in it. It now stays mounted while it holds focus, and carries the same roving tabindex as the other toolbars — one tab stop, Left/Right between controls, wrapping, reversed in RTL. Its +/- format buttons moved from disabled to aria-disabled so they keep their place in that order.

On the tab position: the bar sits between the top bar and the content, which is where the RTE toolbar sits on desktop — Shift+Tab from the text now lands on it. One caveat worth your call: the bar is still only rendered once the content has been focused, so a first forward Tab through the editor passes from the top bar straight to the text. Rendering it earlier would make it visible before the keyboard opens; say the word if you want that instead.

Class sweep: role="toolbar" matches 4 elements in the frontend — EditorToolbar and MobileTopBar were already wired, MobileFormattingBar and LinkBubbleMenu were not. Both are now, so the "left for a follow-up" note is gone from the PR body. I also checked every :disabled and tabindex binding under TipTapEditor/components/: the rest are modal submit buttons and role="menuitem" items, which own their own focus order and are correct as-is.

New specs cover the reachability fix (focus moves from the content into the bar and the bar survives), the single tab stop and arrow movement on both toolbars, and the unavailable format button keeping its place.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

AlexVelezLl · 2026-08-31T21:20:29Z

+      const toolbarRef = ref(null);
+
+      useRovingTabIndex(toolbarRef);


Why do we need this here? it is inside the mobile toolbar, its not a different toolbar.

MobileFormattingBar is a sibling of MobileTopBar, not a child — each is its own role="toolbar" element with its own aria-label, and the composable roves within one container element, so each needs its own call. Without it every button in the bar is a separate tab stop.

They are further apart now: the bar moved after EditorContentWrapper so it follows the content in tab order, matching where it sits on screen.

AlexVelezLl · 2026-08-31T21:30:23Z

+    render(MobileTopBar, {
+      provide: { editor: ref(makeEditorStub()) },
+      router: new VueRouter(),
+    });
+    // Every button, not only the `data-toolbar-item` ones: an unmarked control
+    // would be a second tab stop.
+    const controls = screen.getAllByRole('button');
+
+    expect(controls.length).toBeGreaterThan(1);
+    expect(controls[0]).toHaveAttribute('aria-disabled', 'true');
+    expect(controls[0]).toHaveAttribute('tabindex', '0');
+    controls.slice(1).forEach(control => expect(control).toHaveAttribute('tabindex', '-1'));


Flagging that even if these buttons have a proper tabindex, if I press Tab or Shift + Tab when focusing the editor, the toolbar disappears.

Grabacion.de.pantalla.2026-08-31.a.la.s.4.29.46.p.m.mov

Fixed in b3c5ba4.

Blurring the content scheduled a Vue re-render, and a browser flushes microtasks between blur and the focus landing on the next control — so the bar unmounted before Tab could reach it, and focus fell back into the top bar. Focus is now tracked across the whole .editor-container, cleared only by focusout, the one event that names where focus is going.

The bar also moved after EditorContentWrapper, so Tab from the content steps into it rather than out of the editor; Shift+Tab reaches the top bar.

I searched shared/views/TipTapEditor/ for other mounts driven by editor focus — this v-if was the only one. Three tests in MobileFormattingBar.spec.js cover it; two of them fail against the previous commit.

Roving tabindex over a container's `[data-toolbar-item]` controls, per the WAI-ARIA APG toolbar pattern: one tab stop, Left/Right between controls, wrapping and reversed in RTL. Excludes controls KListWithOverflow has hidden via `visibility`, and ignores arrow keys raised inside an open menu. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rtibblesbot · 2026-08-31T22:35:02Z

Rebased onto unstable and squashed the review rounds into the four commits that ship, so the SHAs cited in the replies above are gone:

1c54e96 / b3c5ba429 (mobile formatting bar reachability) → folded into f469435, alongside the roving tabindex for that bar and LinkBubbleMenu.
The window.isRTL switch and the userEvent test rewrites → folded into the commits that introduced those lines (8023f9f, 51279a5).
78a204c is the one piece that stayed separate: it clears the formatting bar's virtual-keyboard scroll timeout on unmount, which rode along in the reachability fix and is unrelated to it.

One test-only change on top: tabIn moved from __tests__/keyboardHelpers.js to shared/utils/testing.js, the module the repo already uses for helpers shared across specs. Five specs import it; all five updated. No behaviour change — full frontend suite green.

@rtibblesbot's comments are generated by an LLM, and should be evaluated accordingly

Mark every toolbar control `data-toolbar-item` and drive the toolbars with useRovingTabIndex, so Tab moves into the toolbar and then out. Unavailable ToolbarButtons carry `aria-disabled` instead of the native `disabled`, keeping them focusable and in the arrow-key order. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

AlexVelezLl

Last couple of details!

AlexVelezLl · 2026-09-01T12:06:56Z

    <div
      v-if="isExpanded"
      id="formatting-tools"
      class="scrollable-tools"
      :aria-label="textFormattingToolbar$()"
      @touchstart="event => event.stopPropagation()"
      @touchend="event => event.stopPropagation()"


This div seems to be getting an unnecessary tab stop. It seems it's because it's a scrollable div, and Chrome does that. Could you please add a tabindex="-1" here, since it's not necessary and we already have focusable inputs within it; they're just dynamically set.

Added tabindex="-1" with a comment on why.

Swept the branch for other scroll containers holding roving items: grep -rn overflow over TipTapEditor/components/ — .scrollable-tools is the only one. (FormulasMenu also scrolls but is not a roving toolbar.)

AlexVelezLl · 2026-09-01T12:07:54Z

    >
      <button
        class="toggle-btn"
+        data-toolbar-item


These first three buttons do not have a proper focus outline color

Added :focus-visible for .toggle-btn and .format-btn, matching the background: #e6e6e6; outline: 2px solid #0097f2 used elsewhere in the editor. Dropped the shared border-radius: 4px since both already set their own.

Audited every [data-toolbar-item] on the branch against the :focus-visible rules in each file — 10 classes, these two were the only ones missing a rule: .more-button, .link-url, .bubble-menu-button, .toolbar-btn, .format-dropdown, .paste-dropdown-btn and .insert-button were already covered.

AlexVelezLl · 2026-09-01T12:10:29Z

This dropdown menu here does not have a proper accessibility implementation. Could you please use KDropdownMenu here instead?

Replaced it with KDropdownMenu nested in the trigger button, the same shape EditorToolbar's .more-button already uses — KDS takes aria-haspopup/aria-expanded on the trigger, menu semantics, arrow-key navigation and focus return, so the hand-rolled outside-click listener, dropdown markup/styles and the now-unused insertContentOption string are gone. Modal anchoring passes null (centred), since the menu item is gone by the time the handler runs.

Added a MobileTopBar test that opens the menu with Enter and asserts focus lands inside it.

Swept the branch for other hand-rolled menus: grep -rn 'role="menu"' over TipTapEditor/ — FormatDropdown and PasteDropdown also roll their own, but both are outside this PR's diff and unchanged by it, so I left them. Happy to file a follow-up if you want them converted.

AlexVelezLl

Almost there!

AlexVelezLl · 2026-09-01T12:50:23Z

Could you fix this? It seems like the Tiptap editor content, the radio button, and the remove button are on top of the mobile toolbar

Doesn't seem to be introduced by this PR, but now that we are here, it'd be nice to fix :)

Fixed in d72a4fb. ClickableRegion's .content-wrapper carried z-index: 1, which opens a stacking context: the editor's position: fixed children are trapped inside that answer option, and the next option — same z-index, later in the DOM — paints over them. Dropping the z-index keeps the wrapper above .overlay-button (both positioned, wrapper is later in the DOM) and lets the bar's own z-index: 100 count again.

Class check: grepped every z-index under QTIEditor/ and in the three legacy editors that embed the RTE (AnswersEditor, HintsEditor, AssessmentItemEditor). One declaration matched, the one changed. It was also trapping the link, image and math popovers (z-index: 2/10 in TipTapEditor.vue), and it covers all three interaction editors that use ClickableRegion, not just choice.

No test: jsdom has no layout or paint order. Verified by reproducing the nesting in a standalone page in headless Chromium — the later option's radio and text sit over the bar before the change and under it after.

AlexVelezLl · 2026-09-01T12:52:28Z

+        <KDropdownMenu
+          :options="insertOptions"
+          @select="onInsertSelect"
        >
-          <img
-            :src="tool.icon"
-            alt=""
-            class="dropdown-icon"
-            aria-hidden="true"
-          >
-          <span class="dropdown-title">{{ tool.title }}</span>
-        </button>
-      </div>
+          <template #option="{ option }">
+            <div class="insert-option">
+              <img
+                :src="option.icon"
+                alt=""
+                class="dropdown-icon"
+                aria-hidden="true"
+              >
+              <span class="dropdown-title">{{ option.label }}</span>
+            </div>
+          </template>
+        </KDropdownMenu>


Could you fix the styles to match the styles we have on the KListWithOverflow's KDropdownMenu?

Fixed in 3369b91. .insert-option now matches EditorToolbar's .overflow-item — gap: 12px, padding: 8px 12px, font-size: 1.2rem, line-height: 140% — and the icon matches .dropdown-item-icon with flex-shrink: 0 and a 20px box.

Class check: three #option slots in the frontend. EditorToolbar is the KListWithOverflow menu being matched; StudioNavigation's is a nav menu with its own sizing, so this was the only divergent one.

Give MobileFormattingBar and LinkBubbleMenu the same single tab stop as the other two role="toolbar" elements. The formatting bar rendered only while the editor content held focus, so blurring the content scheduled a re-render that unmounted the bar before the browser could land focus on it — Tab and Shift+Tab both made it vanish instead of stepping into it. Track focus across the whole editor container, which only `focusout` — the one event that names where focus is going — can clear. The bar sits below the content on screen, so it now follows it in the DOM too. Its toggle and format-size buttons had no focus-visible outline; nothing had ever focused them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The bar unmounts as soon as the editor loses focus, which can happen inside the 150ms wait for the virtual keyboard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The hand-rolled menu had no menu semantics, no keyboard navigation and no focus return. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

The z-index opened a stacking context, so the mobile formatting bar and the link, image and math popovers were painted over by the next answer option. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

AlexVelezLl

LGTM!

rtibblesbot force-pushed the issue-6104-e657d5 branch 5 times, most recently from 953bca2 to 2871664 Compare August 26, 2026 21:11

rtibblesbot marked this pull request as ready for review August 26, 2026 21:16

rtibblesbot force-pushed the issue-6104-e657d5 branch from 2871664 to ac6bb0b Compare August 26, 2026 21:27

rtibblesbot changed the title ~~Make the rich text editor toolbar a single tab stop with arrow-key navigation~~ Make the rich text editor toolbars a single tab stop with arrow-key navigation Aug 26, 2026

AlexVelezLl requested changes Aug 27, 2026

View reviewed changes

rtibblesbot force-pushed the issue-6104-e657d5 branch from 1c54e96 to 583f4d2 Compare August 27, 2026 15:11

AlexVelezLl requested changes Aug 31, 2026

View reviewed changes

rtibblesbot mentioned this pull request Aug 31, 2026

[RTE] Unify how the TipTap editor determines RTL #6114

Open

3 tasks

rtibblesbot force-pushed the issue-6104-e657d5 branch from b8c1d9a to 643924f Compare August 31, 2026 22:18

rtibblesbot force-pushed the issue-6104-e657d5 branch from 643924f to 78a204c Compare August 31, 2026 22:34

rtibblesbot force-pushed the issue-6104-e657d5 branch from 78a204c to 02fce8f Compare August 31, 2026 22:44

AlexVelezLl requested changes Sep 1, 2026

View reviewed changes

rtibblesbot force-pushed the issue-6104-e657d5 branch from 4cda7be to 3b0aa34 Compare September 1, 2026 12:40

AlexVelezLl requested changes Sep 1, 2026

View reviewed changes

rtibblesbot and others added 4 commits September 1, 2026 06:12

fix: clear the mobile formatting bar's scroll timeout on unmount

0b01359

The bar unmounts as soon as the editor loses focus, which can happen inside the 150ms wait for the virtual keyboard. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

refactor: use KDropdownMenu for the mobile insert menu

efb2199

The hand-rolled menu had no menu semantics, no keyboard navigation and no focus return. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

rtibblesbot force-pushed the issue-6104-e657d5 branch from 3b0aa34 to ae77d4a Compare September 1, 2026 13:14

AlexVelezLl approved these changes Sep 1, 2026

View reviewed changes

AlexVelezLl merged commit 4d00d5d into learningequality:unstable Sep 1, 2026
20 checks passed

rtibblesbot deleted the issue-6104-e657d5 branch September 1, 2026 14:41

Uh oh!

Uh oh!

Conversation

rtibblesbot commented Aug 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

References

Reviewer guidance

AI usage

🟡 Waiting for feedback

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

rtibblesbot commented Aug 27, 2026

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rtibblesbot commented Aug 31, 2026

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AlexVelezLl left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

rtibblesbot commented Aug 26, 2026 •

edited

Loading