Skip to content

Commit 1224909

Browse files
Lara Dvorsekclaude
andcommitted
Exercise variants: make the wizard operable by keyboard
The four adaptation cards in step 1 were divs with a click handler and no tabindex, role or key handling. They are the only way to choose what the variant changes, so a keyboard user could not select anything and the step's Next button stayed disabled — the wizard was unusable without a mouse. Make them real toggle buttons carrying aria-pressed. A button may only contain phrasing content, so the heading, hint and icon wrapper become spans with their own BEM classes, and the SCSS undoes the UA button styling the card design does not want. Keyboard focus now also draws a visible ring. Convert the three href-less disclosure anchors (phase step output, draft warnings, result summary) to buttons with aria-expanded for the same reason. The tray entry keeps role="button" — it hosts the cancel button, so it cannot become a real button — and gains the Space handler that contract requires, suppressing the default so the key does not scroll the page behind the popover. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
1 parent 97730e1 commit 1224909

6 files changed

Lines changed: 116 additions & 28 deletions

File tree

src/main/webapp/app/core/navbar/variant-generation-tray/variant-generation-tray.component.html

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@
5757
data-testid="variant-tray-entry"
5858
(click)="openJobEntry(job)"
5959
(keydown.enter)="openJobEntry(job)"
60+
(keydown.space)="openJobEntryOnSpace($event, job)"
6061
>
6162
<div class="flex items-center justify-between gap-2">
6263
<span class="truncate font-semibold">{{ job.sourceExerciseTitle }}</span>

src/main/webapp/app/core/navbar/variant-generation-tray/variant-generation-tray.component.spec.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,18 @@ describe('VariantGenerationTrayComponent', () => {
153153
expect(routerMock.navigate).not.toHaveBeenCalled();
154154
});
155155

156+
it('opens an entry on Space without scrolling the page behind the tray', () => {
157+
// The entry is a div with role="button" (it hosts the cancel button, so it cannot be a real button).
158+
// That contract requires Space to activate it, and Space must not also scroll the page.
159+
const event = { preventDefault: vi.fn() } as unknown as Event;
160+
161+
component.openJobEntryOnSpace(event, runningJob);
162+
163+
expect(event.preventDefault).toHaveBeenCalled();
164+
expect(component.monitorJobId()).toBe('job-1');
165+
expect(component.monitorVisible()).toBe(true);
166+
});
167+
156168
it('flags failed jobs and drafts with warnings as needing attention', () => {
157169
expect(component.needsAttention(runningJob)).toBe(false);
158170
expect(component.needsAttention(completedJob)).toBe(false);

src/main/webapp/app/core/navbar/variant-generation-tray/variant-generation-tray.component.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,4 +159,16 @@ export class VariantGenerationTrayComponent {
159159
this.monitorJobId.set(job.jobId);
160160
this.monitorVisible.set(true);
161161
}
162+
163+
/**
164+
* Space activation for the entry. The entry carries `role="button"`, so it must respond to Space as well as
165+
* Enter; the default has to be suppressed or the key scrolls the page behind the popover instead.
166+
*
167+
* @param event the keyboard event to suppress
168+
* @param job the job whose entry was activated
169+
*/
170+
openJobEntryOnSpace(event: Event, job: VariantJob): void {
171+
event.preventDefault();
172+
this.openJobEntry(job);
173+
}
162174
}

src/main/webapp/app/course/manage/exercises/create-variant-modal/exercise-variant-ai-modal-wizard.component.html

Lines changed: 58 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -71,58 +71,72 @@
7171
<p class="text-muted-color text-sm mb-4">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.intro' | artemisTranslate }}</p>
7272

7373
<div class="option-cards mb-6">
74-
<div class="option-card" [class.option-card--selected]="changeDifficulty()" (click)="toggleDifficulty()" data-testid="variant-option-difficulty">
74+
<button
75+
type="button"
76+
class="option-card"
77+
[class.option-card--selected]="changeDifficulty()"
78+
[attr.aria-pressed]="changeDifficulty()"
79+
(click)="toggleDifficulty()"
80+
data-testid="variant-option-difficulty"
81+
>
7582
@if (changeDifficulty()) {
7683
<span class="option-card__check"><fa-icon [icon]="faCheck" /></span>
7784
}
78-
<div class="option-card__icon"><fa-icon [icon]="faGaugeHigh" /></div>
79-
<h6>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.difficulty' | artemisTranslate }}</h6>
80-
<p>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.difficultyHint' | artemisTranslate }}</p>
85+
<span class="option-card__icon"><fa-icon [icon]="faGaugeHigh" /></span>
86+
<span class="option-card__title">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.difficulty' | artemisTranslate }}</span>
87+
<span class="option-card__hint">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.difficultyHint' | artemisTranslate }}</span>
8188
<span class="option-card__cta">
8289
@if (changeDifficulty()) {
8390
{{ 'artemisApp.exerciseVariantGeneration.wizard.selected' | artemisTranslate }}
8491
}
8592
</span>
86-
</div>
87-
<div class="option-card" [class.option-card--selected]="changeDomain()" (click)="toggleDomain()">
93+
</button>
94+
<button type="button" class="option-card" [class.option-card--selected]="changeDomain()" [attr.aria-pressed]="changeDomain()" (click)="toggleDomain()">
8895
@if (changeDomain()) {
8996
<span class="option-card__check"><fa-icon [icon]="faCheck" /></span>
9097
}
91-
<div class="option-card__icon"><fa-icon [icon]="faEarthAmericas" /></div>
92-
<h6>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.domain' | artemisTranslate }}</h6>
93-
<p>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.domainHint' | artemisTranslate }}</p>
98+
<span class="option-card__icon"><fa-icon [icon]="faEarthAmericas" /></span>
99+
<span class="option-card__title">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.domain' | artemisTranslate }}</span>
100+
<span class="option-card__hint">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.domainHint' | artemisTranslate }}</span>
94101
<span class="option-card__cta">
95102
@if (changeDomain()) {
96103
{{ 'artemisApp.exerciseVariantGeneration.wizard.selected' | artemisTranslate }}
97104
}
98105
</span>
99-
</div>
100-
<div class="option-card" [class.option-card--selected]="changeNarrative()" (click)="toggleNarrative()" data-testid="variant-option-narrative">
106+
</button>
107+
<button
108+
type="button"
109+
class="option-card"
110+
[class.option-card--selected]="changeNarrative()"
111+
[attr.aria-pressed]="changeNarrative()"
112+
(click)="toggleNarrative()"
113+
data-testid="variant-option-narrative"
114+
>
101115
@if (changeNarrative()) {
102116
<span class="option-card__check"><fa-icon [icon]="faCheck" /></span>
103117
}
104-
<div class="option-card__icon"><fa-icon [icon]="faBookOpen" /></div>
105-
<h6>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.narrative' | artemisTranslate }}</h6>
106-
<p>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.narrativeHint' | artemisTranslate }}</p>
118+
<span class="option-card__icon"><fa-icon [icon]="faBookOpen" /></span>
119+
<span class="option-card__title">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.narrative' | artemisTranslate }}</span>
120+
<span class="option-card__hint">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.narrativeHint' | artemisTranslate }}</span>
107121
<span class="option-card__cta">
108122
@if (changeNarrative()) {
109123
{{ 'artemisApp.exerciseVariantGeneration.wizard.selected' | artemisTranslate }}
110124
}
111125
</span>
112-
</div>
113-
<div class="option-card" [class.option-card--selected]="changeCustom()" (click)="toggleCustom()">
126+
</button>
127+
<button type="button" class="option-card" [class.option-card--selected]="changeCustom()" [attr.aria-pressed]="changeCustom()" (click)="toggleCustom()">
114128
@if (changeCustom()) {
115129
<span class="option-card__check"><fa-icon [icon]="faCheck" /></span>
116130
}
117-
<div class="option-card__icon"><fa-icon [icon]="faPenToSquare" /></div>
118-
<h6>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.custom' | artemisTranslate }}</h6>
119-
<p>{{ 'artemisApp.exerciseVariantGeneration.wizard.select.customHint' | artemisTranslate }}</p>
131+
<span class="option-card__icon"><fa-icon [icon]="faPenToSquare" /></span>
132+
<span class="option-card__title">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.custom' | artemisTranslate }}</span>
133+
<span class="option-card__hint">{{ 'artemisApp.exerciseVariantGeneration.wizard.select.customHint' | artemisTranslate }}</span>
120134
<span class="option-card__cta">
121135
@if (changeCustom()) {
122136
{{ 'artemisApp.exerciseVariantGeneration.wizard.selected' | artemisTranslate }}
123137
}
124138
</span>
125-
</div>
139+
</button>
126140
</div>
127141

128142
<div class="flex justify-end mt-8">
@@ -397,10 +411,16 @@ <h6 class="font-semibold mb-6">{{ 'artemisApp.exerciseVariantGeneration.wizard.g
397411
field, oldest first (latest at the bottom) — earlier attempt errors stay
398412
inspectable after a later attempt succeeded. -->
399413
@if (hasAnyDetail(outputs)) {
400-
<a class="block cursor-pointer text-sm no-underline" (click)="toggleStepOutput(phase)" data-testid="variant-wizard-step-output-toggle">
414+
<button
415+
type="button"
416+
class="block w-full cursor-pointer border-0 bg-transparent p-0 text-start no-underline text-sm"
417+
[attr.aria-expanded]="!!expandedPhases()[phase]"
418+
(click)="toggleStepOutput(phase)"
419+
data-testid="variant-wizard-step-output-toggle"
420+
>
401421
<fa-icon [icon]="expandedPhases()[phase] ? faChevronDown : faChevronRight" class="mr-1" />
402422
{{ outputs[outputs.length - 1].summary }}
403-
</a>
423+
</button>
404424
@if (expandedPhases()[phase]) {
405425
@for (stepOutput of outputs; track $index) {
406426
@if (stepOutput.detail) {
@@ -496,10 +516,16 @@ <h6 class="font-semibold mb-6">{{ 'artemisApp.exerciseVariantGeneration.wizard.g
496516
<div class="min-w-0">
497517
<div class="font-semibold">{{ 'artemisApp.exerciseVariantGeneration.wizard.draftWithWarnings' | artemisTranslate }}</div>
498518
@if (warnings().length > 0) {
499-
<a class="block cursor-pointer text-sm no-underline" (click)="toggleWarnings()" data-testid="variant-wizard-warnings-toggle">
519+
<button
520+
type="button"
521+
class="block w-full cursor-pointer border-0 bg-transparent p-0 text-start no-underline text-sm"
522+
[attr.aria-expanded]="warningsExpanded()"
523+
(click)="toggleWarnings()"
524+
data-testid="variant-wizard-warnings-toggle"
525+
>
500526
<fa-icon [icon]="warningsExpanded() ? faChevronDown : faChevronRight" class="mr-1" />
501527
{{ 'artemisApp.exerciseVariantGeneration.wizard.warningDetails' | artemisTranslate: { count: warnings().length } }}
502-
</a>
528+
</button>
503529
@if (warningsExpanded()) {
504530
@for (warning of warnings(); track warning) {
505531
<pre class="step-output-detail" data-testid="variant-wizard-warning-detail">{{ warning }}</pre>
@@ -568,10 +594,16 @@ <h6 class="font-semibold mb-6">{{ 'artemisApp.exerciseVariantGeneration.wizard.g
568594
<!-- Collapsed: the latest message's summary. Expanded: the phase's FULL history, each
569595
message in its own scrollable code field, latest at the bottom. -->
570596
@if (hasAnyDetail(entry.outputs)) {
571-
<a class="block cursor-pointer no-underline" (click)="toggleStepOutput(entry.phase)" data-testid="variant-wizard-summary-toggle">
597+
<button
598+
type="button"
599+
class="block w-full cursor-pointer border-0 bg-transparent p-0 text-start no-underline"
600+
[attr.aria-expanded]="!!expandedPhases()[entry.phase]"
601+
(click)="toggleStepOutput(entry.phase)"
602+
data-testid="variant-wizard-summary-toggle"
603+
>
572604
<fa-icon [icon]="expandedPhases()[entry.phase] ? faChevronDown : faChevronRight" class="mr-1" />
573605
{{ entry.outputs[entry.outputs.length - 1].summary }}
574-
</a>
606+
</button>
575607
@if (expandedPhases()[entry.phase]) {
576608
@for (stepOutput of entry.outputs; track $index) {
577609
@if (stepOutput.detail) {

src/main/webapp/app/course/manage/exercises/create-variant-modal/exercise-variant-ai-modal-wizard.component.scss

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -136,6 +136,14 @@
136136

137137
.option-card {
138138
position: relative;
139+
140+
// A real <button> so the cards are focusable and operable by keyboard; these declarations undo the
141+
// UA button styling the card design does not want.
142+
display: block;
143+
width: 100%;
144+
margin: 0;
145+
font: inherit;
146+
color: inherit;
139147
border: 1px solid var(--p-content-border-color);
140148
border-radius: 0.75rem;
141149
padding: 1.25rem 1rem;
@@ -146,6 +154,11 @@
146154
user-select: none;
147155
overflow: hidden;
148156

157+
&:focus-visible {
158+
outline: 2px solid var(--primary);
159+
outline-offset: 2px;
160+
}
161+
149162
&::before {
150163
content: '';
151164
position: absolute;
@@ -185,13 +198,15 @@
185198
transform: translateY(-2px);
186199
}
187200

188-
h6 {
201+
&__title {
202+
display: block;
189203
font-weight: 600;
190204
font-size: 0.87rem;
191205
margin-bottom: 0.3rem;
192206
}
193207

194-
p {
208+
&__hint {
209+
display: block;
195210
font-size: 0.75rem;
196211
color: var(--p-text-muted-color);
197212
line-height: 1.35;

src/main/webapp/app/course/manage/exercises/create-variant-modal/exercise-variant-ai-modal-wizard.component.spec.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,22 @@ describe('ExerciseVariantAiModalWizardComponent (storytelling)', () => {
279279
TestBed.resetTestingModule();
280280
});
281281

282+
it('exposes the adaptation cards as real toggle buttons so they can be operated without a mouse', () => {
283+
const card = document.body.querySelector('[data-testid="variant-option-narrative"]');
284+
285+
// A native button is focusable and fires (click) on both Enter and Space; a plain div does neither, which
286+
// left step 1 impossible to complete without a mouse and the Next button permanently disabled.
287+
expect(card?.tagName).toBe('BUTTON');
288+
expect(card?.getAttribute('type')).toBe('button');
289+
expect(card?.getAttribute('aria-pressed')).toBe('false');
290+
291+
(card as HTMLButtonElement).click();
292+
fixture.detectChanges();
293+
294+
expect(component.changeNarrative()).toBe(true);
295+
expect(document.body.querySelector('[data-testid="variant-option-narrative"]')?.getAttribute('aria-pressed')).toBe('true');
296+
});
297+
282298
it('offers the storytelling option card and accepts it as the only selection', () => {
283299
expect(document.body.querySelector('[data-testid="variant-option-narrative"]')).not.toBeNull();
284300

0 commit comments

Comments
 (0)