Skip to content

Make the rich text editor toolbars a single tab stop with arrow-key navigation #6108

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
AlexVelezLl merged 6 commits into from
Sep 1, 2026
Merged

Make the rich text editor toolbars a single tab stop with arrow-key navigation #6108

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8023f9f
feat: add useRovingTabIndex composable
rtibblesbot Aug 31, 2026
042bd9b
feat: make the rich text editor toolbars a single tab stop
rtibblesbot Aug 31, 2026
a0b0c66
feat: extend the toolbar roving tabindex to the mobile and link toolbars
rtibblesbot Aug 31, 2026
0b01359
fix: clear the mobile formatting bar's scroll timeout on unmount
rtibblesbot Aug 31, 2026
efb2199
refactor: use KDropdownMenu for the mobile insert menu
rtibblesbot Sep 1, 2026
ae77d4a
fix: keep the clickable region from trapping the editor's fixed overlays
rtibblesbot Sep 1, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions contentcuration/contentcuration/frontend/shared/utils/testing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,3 +25,18 @@ export async function resetMockChannelScope() {
Session.currentChannelId = Session._oldCurrentChannelId;
delete Session._oldCurrentChannelId;
}

/**
* Tab into the component under test, entering backwards from a sentinel after it.
*
* Tabbing forward from the start of the document stops on the CSRF input the
* shared Jest setup leaves at the top of the body.
*
* @param {import('@testing-library/user-event').UserEvent} user
*/
export async function tabIn(user) {
const sentinel = document.body.appendChild(document.createElement('button'));
sentinel.focus();
await user.tab({ shift: true });
sentinel.remove();
}
4 changes: 3 additions & 1 deletion ...tion/contentcuration/frontend/shared/views/QTIEditor/components/ClickableRegion/index.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,9 +85,11 @@
}
}

/* No z-index: the stacking context it opens would trap the fixed toolbars and
popovers of an editor in the slot. Being positioned and later in the DOM
already paints this above the overlay button. */
.content-wrapper {
position: relative;
z-index: 1;
}

</style>
36 changes: 25 additions & 11 deletions ...curation/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/TipTapEditor.vue
View file
Open in desktop
Comment thread
AlexVelezLl marked this conversation as resolved.
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
:aria-label="editorMode === 'edit' ? TipTapEditorLabel$() : TipTapViewerLabel$()"
aria-multiline="true"
@keydown="handleContainerKeydown"
@focusin="hasFocusWithin = true"
@focusout="handleFocusout"
>
<div v-if="editorMode === 'edit'">
<EditorToolbar
Expand All @@ -21,16 +23,11 @@
@minimize="emitMinimize"
/>

<div v-else>
<MobileTopBar
v-on="sharedEventHandlers"
@minimize="emitMinimize"
/>
<MobileFormattingBar
v-if="isFocused"
v-on="sharedEventHandlers"
/>
</div>
<MobileTopBar
v-else
v-on="sharedEventHandlers"
@minimize="emitMinimize"
/>
</div>

<div
Expand Down Expand Up @@ -97,6 +94,13 @@
@drop.native.prevent="handleDrop"
@dragover.native.prevent
/>

<!-- After the content area: the bar is pinned below it on screen, so Tab out of
the content should reach it rather than leave the editor. -->
<MobileFormattingBar
v-if="isTouchDevice && editorMode === 'edit' && hasFocusWithin"
v-on="sharedEventHandlers"
/>
</div>

</template>
Expand Down Expand Up @@ -167,6 +171,15 @@
'insert-math': target => mathHandler.openCreateMathModal({ targetElement: target }),
}));

// Tracked on the container rather than on the editor content: tabbing out of
// the content blurs it, and the re-render that blur schedules would unmount
// the mobile formatting bar before focus could land on it. `focusout` is the
// only signal that names where focus is going, so it alone clears this.
const hasFocusWithin = ref(false);
const handleFocusout = event => {
hasFocusWithin.value = editorContainer.value.contains(event.relatedTarget);
};

const handleDrop = event => {
const file = event.dataTransfer.files[0];
if (file) {
Expand Down Expand Up @@ -278,7 +291,8 @@
return {
editorContainer,
isReady,
isFocused,
hasFocusWithin,
handleFocusout,
handleDrop,
linkHandler,
editor,
Expand Down
4 changes: 0 additions & 4 deletions ...on/contentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/TipTapEditorStrings.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -383,10 +383,6 @@ const MESSAGES = {
message: 'Insert content menu',
context: 'Accessibility label for the insert content menu button',
},
insertContentOption: {
message: 'Insert content option',
context: 'Accessibility label for the insert content dropdown menu',
},
};

let TipTapEditorStrings = null;
Expand Down
3 changes: 3 additions & 0 deletions ...tentcuration/frontend/shared/views/TipTapEditor/TipTapEditor/components/EditorToolbar.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@
<template #more="{ overflowItems }">
<button
class="more-button"
data-toolbar-item
:class="
$computedClass({
':is([aria-expanded=\'true\'])': {
Expand Down Expand Up @@ -142,6 +143,7 @@
import { useToolbarActions } from '../composables/useToolbarActions';
import { getTipTapEditorStrings } from '../TipTapEditorStrings';
import { useDropdowns } from '../composables/useDropdowns';
import { useRovingTabIndex } from '../composables/useRovingTabIndex';
import ToolbarButton from './toolbar/ToolbarButton.vue';
import FormatDropdown from './toolbar/FormatDropdown.vue';
import PasteDropdown from './toolbar/PasteDropdown.vue';
Expand All @@ -157,6 +159,7 @@
},
setup(props, { emit }) {
const toolbarRef = ref(null);
useRovingTabIndex(toolbarRef);

const {
handleCopy,
Expand Down
13 changes: 12 additions & 1 deletion ...ration/frontend/shared/views/TipTapEditor/TipTapEditor/components/link/LinkBubbleMenu.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<template>

<div
ref="toolbarRef"
class="link-bubble-menu"
role="toolbar"
:aria-label="linkActions$()"
Expand All @@ -9,6 +10,7 @@
:href="href"
target="_blank"
class="link-url"
data-toolbar-item
:aria-label="`${goToLink$()} ${opensInNewTab$()}`"
>
{{ goToLink$() }}
Expand All @@ -22,6 +24,7 @@

<button
class="bubble-menu-button"
data-toolbar-item
:title="copyLink$()"
:aria-label="copyLink$()"
@click="copyToClipboard(href)"
Expand All @@ -35,6 +38,7 @@

<button
class="bubble-menu-button"
data-toolbar-item
:title="editLink$()"
:aria-label="editLink$()"
@click="onEdit"
Expand All @@ -48,6 +52,7 @@

<button
class="bubble-menu-button"
data-toolbar-item
:title="removeLink$()"
:aria-label="removeLink$()"
@click="onRemove"
Expand All @@ -65,12 +70,17 @@

<script>

import { defineComponent, computed, inject } from 'vue';
import { defineComponent, computed, inject, ref } from 'vue';
import { getTipTapEditorStrings } from '../../TipTapEditorStrings';
import { useRovingTabIndex } from '../../composables/useRovingTabIndex';

export default defineComponent({
name: 'LinkBubbleMenu',
setup(props) {
const toolbarRef = ref(null);

useRovingTabIndex(toolbarRef);

const { goToLink$, copyLink$, editLink$, removeLink$, linkActions$, opensInNewTab$ } =
getTipTapEditorStrings();

Expand All @@ -82,6 +92,7 @@
};

return {
toolbarRef,
href,
onEdit: () => openLinkEditor('edit'),
onRemove: removeLink,
Expand Down
1 change: 1 addition & 0 deletions ...ion/frontend/shared/views/TipTapEditor/TipTapEditor/components/toolbar/FormatDropdown.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
<button
ref="dropdownButton"
class="format-dropdown"
data-toolbar-item
:aria-expanded="isOpen"
:aria-haspopup="true"
:aria-label="textFormatOptions$()"
Expand Down
27 changes: 23 additions & 4 deletions ...rontend/shared/views/TipTapEditor/TipTapEditor/components/toolbar/MobileFormattingBar.vue
View file
Open in desktop

@AlexVelezLl AlexVelezLl Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Image

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

@rtibblesbot rtibblesbot Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
<template>

<div
ref="toolbarRef"
class="floating-panel"
role="toolbar"
:aria-label="textFormattingToolbar$()"
Expand All @@ -12,6 +13,7 @@
>
<button
class="toggle-btn"
data-toolbar-item

@AlexVelezLl AlexVelezLl Sep 1, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Image

@rtibblesbot rtibblesbot Sep 1, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

:title="isExpanded ? collapseFormattingBar$() : expandFormattingBar$()"
:aria-label="isExpanded ? collapseFormattingBar$() : expandFormattingBar$()"
:aria-expanded="isExpanded"
Expand All @@ -22,11 +24,14 @@
{{ isExpanded ? '×' : '+' }}
</button>
</div>
<!-- Chrome gives a scroll container its own tab stop once the roving tabindex
has left every child at -1. -->
<div
v-if="isExpanded"
id="formatting-tools"
class="scrollable-tools"
:aria-label="textFormattingToolbar$()"
tabindex="-1"
@touchstart="event => event.stopPropagation()"
@touchend="event => event.stopPropagation()"
>
Expand All @@ -36,7 +41,8 @@
:aria-label="formatSize$()"
>
<button
:disabled="!canDecreaseFormat"
data-toolbar-item
:aria-disabled="canDecreaseFormat ? 'false' : 'true'"
:title="decreaseFormatSize$()"
:aria-label="decreaseFormatSize$()"
class="format-btn"
Expand All @@ -51,7 +57,8 @@
aria-hidden="true"
>
<button
:disabled="!canIncreaseFormat"
data-toolbar-item
:aria-disabled="canIncreaseFormat ? 'false' : 'true'"
:title="increaseFormatSize$()"
:aria-label="increaseFormatSize$()"
class="format-btn"
Expand Down Expand Up @@ -122,6 +129,7 @@
import { useToolbarActions } from '../../composables/useToolbarActions';
import { useFormatControls } from '../../composables/useFormatControls';
import { getTipTapEditorStrings } from '../../TipTapEditorStrings';
import { useRovingTabIndex } from '../../composables/useRovingTabIndex';
import ToolbarButton from './ToolbarButton.vue';
import ToolbarDivider from './ToolbarDivider.vue';

Expand All @@ -132,6 +140,9 @@
const isExpanded = ref(true);
const keyboardOffset = ref(0);
const editor = inject('editor');
const toolbarRef = ref(null);

useRovingTabIndex(toolbarRef);
Comment on lines +143 to +145

@AlexVelezLl AlexVelezLl Aug 31, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@rtibblesbot rtibblesbot Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


const {
collapseFormattingBar$,
Expand All @@ -155,13 +166,14 @@
onMounted(() => {
if (editor.value) {
// Use a timeout to allow the keyboard to start appearing
setTimeout(() => {
const scrollTimeout = setTimeout(() => {
const { from } = editor.value.state.selection;
editor.value.view.dom.querySelector(`[pos="${from}"]`)?.scrollIntoView({
behavior: 'smooth',
block: 'nearest',
});
}, 150);
onUnmounted(() => clearTimeout(scrollTimeout));
}

const vk = navigator.virtualKeyboard;
Expand Down Expand Up @@ -207,6 +219,7 @@
return {
isExpanded,
keyboardOffset,
toolbarRef,
textActions,
listActions,
insertTools,
Expand Down Expand Up @@ -306,12 +319,18 @@
border-radius: 0.25rem;
}

.format-btn:disabled {
.format-btn[aria-disabled='true'] {
color: #d1d5da;
cursor: not-allowed;
border-color: #e1e5e9;
}

.toggle-btn:focus-visible,
.format-btn:focus-visible {
background: #e6e6e6;
outline: 2px solid #0097f2;
}

.scrollable-tools {
display: flex;
flex-grow: 1;
Expand Down
Loading
Loading