-
-
Notifications
You must be signed in to change notification settings - Fork 303
feat: introduce ClickableRegion component to improve keyboard accessibility for interaction editors #6094
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.
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
base: unstable
Are you sure you want to change the base?
feat: introduce ClickableRegion component to improve keyboard accessibility for interaction editors #6094
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,69 @@ | ||
| import { render, screen, fireEvent } from '@testing-library/vue'; | ||
| import VueRouter from 'vue-router'; | ||
| import ClickableRegion from '../index.vue'; | ||
|
|
||
| describe('ClickableRegion', () => { | ||
| it('renders a button with the given aria-label', () => { | ||
| render(ClickableRegion, { | ||
| props: { | ||
| ariaLabel: 'Test label', | ||
| }, | ||
| routes: new VueRouter(), | ||
| }); | ||
|
|
||
| expect(screen.getByRole('button', { name: 'Test label' })).toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('does not render the button when suppressed is true', () => { | ||
| render(ClickableRegion, { | ||
| props: { | ||
| ariaLabel: 'Test label', | ||
| suppressed: true, | ||
| }, | ||
| routes: new VueRouter(), | ||
| }); | ||
|
|
||
| expect(screen.queryByRole('button')).not.toBeInTheDocument(); | ||
| }); | ||
|
|
||
| it('emits a single click event on mouse click', async () => { | ||
| const { emitted } = render(ClickableRegion, { | ||
| props: { | ||
| ariaLabel: 'Test label', | ||
| }, | ||
| routes: new VueRouter(), | ||
| }); | ||
|
|
||
| await fireEvent.click(screen.getByRole('button')); | ||
|
|
||
| expect(emitted().click).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('emits a single click event on Enter key', async () => { | ||
| const { emitted } = render(ClickableRegion, { | ||
| props: { | ||
| ariaLabel: 'Test label', | ||
| }, | ||
| routes: new VueRouter(), | ||
| }); | ||
|
|
||
| const button = screen.getByRole('button'); | ||
| await fireEvent.keyDown(button, { key: 'Enter', code: 'Enter' }); | ||
|
|
||
| expect(emitted().click).toHaveLength(1); | ||
| }); | ||
|
|
||
| it('emits a single click event on Space key', async () => { | ||
| const { emitted } = render(ClickableRegion, { | ||
| props: { | ||
| ariaLabel: 'Test label', | ||
| }, | ||
| routes: new VueRouter(), | ||
| }); | ||
|
|
||
| const button = screen.getByRole('button'); | ||
| await fireEvent.keyDown(button, { key: ' ', code: 'Space' }); | ||
|
|
||
| expect(emitted().click).toHaveLength(1); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,96 @@ | ||
| <template> | ||
|
|
||
| <div | ||
| class="clickable-area" | ||
| @click="onClick" | ||
| > | ||
| <button | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is working great for keyboard navigation, but now for click interactions on prompts (this does not happen on options), we should have a slight background color change on hover, and we should have a cursor: pointer. Grabacion.de.pantalla.2026-08-19.a.la.s.4.33.08.a.m.mov |
||
| v-if="!suppressed" | ||
| type="button" | ||
| class="overlay-button" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, another small thing: the focus outline for this is blue-ish, but it should be the same color we have on $coreOutline Outline color now:
Correct outline color
|
||
| :aria-label="ariaLabel" | ||
| @keydown.enter.prevent="onClick" | ||
| @keydown.space.prevent="onClick" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If we add |
||
| ></button> | ||
| <div class="content-wrapper"> | ||
| <slot></slot> | ||
| </div> | ||
| </div> | ||
|
|
||
| </template> | ||
|
|
||
|
|
||
| <script> | ||
|
|
||
| export default { | ||
| name: 'ClickableRegion', | ||
| setup(props, { emit }) { | ||
| function onClick(event) { | ||
| if (props.suppressed) return; | ||
| if (event && event.target && event.target.closest) { | ||
| if ( | ||
| event.target.closest( | ||
| 'button:not(.overlay-button), input, a, select, textarea, [role="button"]', | ||
| ) | ||
| ) { | ||
| return; | ||
| } | ||
| } | ||
| emit('click', event); | ||
| } | ||
| return { onClick }; | ||
| }, | ||
| props: { | ||
| ariaLabel: { | ||
| type: String, | ||
| required: true, | ||
| }, | ||
| suppressed: { | ||
| type: Boolean, | ||
| default: false, | ||
| }, | ||
| }, | ||
| emits: ['click'], | ||
| }; | ||
|
|
||
| </script> | ||
|
|
||
|
|
||
| <style lang="scss" scoped> | ||
|
|
||
| .clickable-area { | ||
| position: relative; | ||
| border-radius: inherit; | ||
| } | ||
|
|
||
| .overlay-button { | ||
| position: absolute; | ||
| top: 0; | ||
| left: 0; | ||
| z-index: 0; | ||
| width: 100%; | ||
| height: 100%; | ||
| padding: 0; | ||
| margin: 0; | ||
| cursor: pointer; | ||
| background: transparent; | ||
| border: 0; | ||
| border-radius: inherit; | ||
| outline: none; | ||
|
|
||
| &:hover { | ||
| background-color: v-bind('$themeTokens.fineLine'); | ||
| } | ||
|
|
||
| &:focus-visible { | ||
| outline: 2px solid v-bind('$themeTokens.primary'); | ||
| outline-offset: 2px; | ||
| } | ||
| } | ||
|
|
||
| .content-wrapper { | ||
| position: relative; | ||
| z-index: 1; | ||
| } | ||
|
|
||
| </style> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -27,9 +27,11 @@ | |
| </div> | ||
|
|
||
| <!-- Prompt --> | ||
| <div | ||
| <ClickableRegion | ||
| :class="promptWrapperClass" | ||
| :style="promptWrapperStyle" | ||
| :suppressed="mode !== 'edit' || isQuestionOpen" | ||
| :aria-label="editQuestionLabel$()" | ||
| @click="handlePromptClick" | ||
| > | ||
| <div class="choice-card-text is-closed"> | ||
|
|
@@ -42,14 +44,15 @@ | |
| :minHeight="'80px'" | ||
| :autofocus="mode === 'edit' && isQuestionOpen" | ||
| :imageProcessor="EditorImageProcessor" | ||
| :tabindex="isQuestionOpen ? 0 : -1" | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need these tab indexes; if it's open, tabindex should also be -1 because we can already navigate through the editor to the input. (i.e., it should always be -1, as we are already handling the tab index in the outer container). |
||
| class="editor" | ||
| @update="setPrompt" | ||
| @minimize="closeQuestion" | ||
| /> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </div> | ||
| </ClickableRegion> | ||
| </div> | ||
|
|
||
| <!-- Choice list --> | ||
|
|
@@ -90,10 +93,12 @@ | |
| class="choice-group" | ||
| > | ||
| <!-- Bordered choice card --> | ||
| <div | ||
| <ClickableRegion | ||
| class="choice-border" | ||
| :class="getChoiceClasses(choice)" | ||
| :style="getChoiceStyle(choice)" | ||
| :suppressed="mode !== 'edit' || isChoiceOpen(choice.id)" | ||
| :aria-label="editAnswerOptionLabel$({ number: index + 1 })" | ||
| @click="handleChoiceClick($event, choice.id)" | ||
| > | ||
| <div | ||
|
|
@@ -152,6 +157,7 @@ | |
| :minHeight="'80px'" | ||
| :autofocus="isChoiceOpen(choice.id)" | ||
| :imageProcessor="EditorImageProcessor" | ||
| :tabindex="isChoiceOpen(choice.id) ? 0 : -1" | ||
| class="editor" | ||
| @update="html => setChoiceContent(choice.id, html)" | ||
| @minimize="closeChoice" | ||
|
|
@@ -181,7 +187,7 @@ | |
| > | ||
| {{ errorDuplicateChoiceContent$() }} | ||
| </ValidationMessage> | ||
| </div> | ||
| </ClickableRegion> | ||
| </div> | ||
| </component> | ||
|
|
||
|
|
@@ -211,6 +217,7 @@ | |
| import CollapsibleToolbar from '../../components/CollapsibleToolbar/index.vue'; | ||
| import ValidationMessage from '../../components/ValidationMessage/index.vue'; | ||
| import AddListItemButton from '../../components/AddListItemButton/index.vue'; | ||
| import ClickableRegion from '../../components/ClickableRegion/index.vue'; | ||
| import AnswerSettings from './components/AnswerSettings/index.vue'; | ||
| import TipTapEditor from 'shared/views/TipTapEditor/TipTapEditor/TipTapEditor'; | ||
| import EditorImageProcessor from 'shared/views/TipTapEditor/TipTapEditor/services/imageService'; | ||
|
|
@@ -219,6 +226,7 @@ | |
| name: 'ChoiceInteractionEditor', | ||
|
|
||
| components: { | ||
| ClickableRegion, | ||
| TipTapEditor, | ||
| CollapsibleToolbar, | ||
| ValidationMessage, | ||
|
|
@@ -245,6 +253,8 @@ | |
| answersLabel$, | ||
| answersDescriptionSingleChoice$, | ||
| answersDescriptionMultipleChoice$, | ||
| editQuestionLabel$, | ||
| editAnswerOptionLabel$, | ||
| } = qtiEditorStrings; | ||
|
|
||
| const palette = themePalette(); | ||
|
|
@@ -280,18 +290,28 @@ | |
|
|
||
| function handlePromptClick(event) { | ||
| if (props.mode !== 'edit') return; | ||
| if (event.target.closest('button') || event.target.closest('input')) return; | ||
| const closestBtn = | ||
| event.target && event.target.closest ? event.target.closest('button') : null; | ||
| if (closestBtn && !closestBtn.classList.contains('overlay-button')) return; | ||
| const closestInput = | ||
| event.target && event.target.closest ? event.target.closest('input') : null; | ||
| if (closestInput) return; | ||
| if (!isQuestionOpen.value) { | ||
| event.stopPropagation(); | ||
| if (event && event.stopPropagation) event.stopPropagation(); | ||
| openQuestion(); | ||
| } | ||
| } | ||
|
Comment on lines
291
to
303
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need all of this? It's even less needed now because when the editor is already open, the Clickable region cannot fire any click event anymore, right? |
||
|
|
||
| function handleChoiceClick(event, choiceId) { | ||
| if (props.mode !== 'edit') return; | ||
| if (openChoiceId.value === choiceId) return; | ||
| if (event.target.closest('button') || event.target.closest('input')) return; | ||
| event.stopPropagation(); | ||
| const closestBtn = | ||
| event.target && event.target.closest ? event.target.closest('button') : null; | ||
| if (closestBtn && !closestBtn.classList.contains('overlay-button')) return; | ||
| const closestInput = | ||
| event.target && event.target.closest ? event.target.closest('input') : null; | ||
| if (closestInput) return; | ||
| if (event && event.stopPropagation) event.stopPropagation(); | ||
| openChoice(choiceId); | ||
| } | ||
|
|
||
|
|
@@ -433,7 +453,7 @@ | |
| } | ||
| return { | ||
| borderColor: questionHasError.value ? tokens.error : tokens.fineLine, | ||
| cursor: props.mode === 'edit' ? 'pointer' : undefined, | ||
| '--clickable-region-hover-bg': palette.blue.v_100, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I don't think we are using this anywhere. |
||
| }; | ||
| }); | ||
|
|
||
|
|
@@ -475,9 +495,12 @@ | |
| borderColor = palette.green.v_500; | ||
| } | ||
|
|
||
| const hoverBg = isCorrect ? palette.green.v_100 : tokens.fineLine; | ||
|
|
||
| return { | ||
| borderColor, | ||
| backgroundColor: isCorrect ? palette.green.v_50 : null, | ||
| '--clickable-region-hover-bg': hoverBg, | ||
| }; | ||
| } | ||
|
|
||
|
|
@@ -524,6 +547,8 @@ | |
| errorEmptyChoiceContent$, | ||
| errorDuplicateChoiceContent$, | ||
| questionLabel$, | ||
| editQuestionLabel$, | ||
| editAnswerOptionLabel$, | ||
| }; | ||
| }, | ||
|
|
||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
@clickshould be on thebuttonelement.