Skip to content

Commit 4049397

Browse files
committed
Improves Settings control accessibility and match highlighting
Announce the slider's unit via wa-slider's valueFormatter, surface the switch's disabled reason through wa-switch's hint (aria-describedby), replace the token menu's listbox/option misuse with a labeled group of buttons, and use the shared sr-only helper in the nav. Also highlight only the first visible variant of a paired control so duplicate-key descriptors don't double-highlight.
1 parent c54701b commit 4049397

5 files changed

Lines changed: 27 additions & 18 deletions

File tree

src/webviews/apps/settings/components/format-input.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -430,16 +430,17 @@ export class GlFormatInput extends SignalWatcher(LitElement) {
430430
>
431431
<code-icon icon="chevron-down" aria-hidden="true"></code-icon>
432432
</button>
433-
<div slot="content" class="tokens" role="listbox" aria-label="Available tokens">
433+
<div slot="content" class="tokens" role="group" aria-label="Available tokens">
434434
<h3 class="tokens__title">Insert token</h3>
435435
${tokens.map(t => {
436436
// Commit tokens insert wrapped in `${…}`; date tokens insert bare
437437
// eslint-disable-next-line prefer-template -- a template literal would need `\${` escaping, which is harder to read
438438
const text = tokenMode === 'date' ? t.token : '${' + t.token + '}';
439+
// Plain action buttons (insert on click) — not listbox options,
440+
// which would imply arrow-key selection semantics
439441
return html`<button
440442
type="button"
441443
class="token"
442-
role="option"
443444
@click=${() => this.insertToken(text)}
444445
>
445446
<code>${text}</code><span>${t.label}</span>

src/webviews/apps/settings/components/settings-detail.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -231,7 +231,16 @@ export class GlSettingsDetail extends SignalWatcher(LitElement) {
231231
*/
232232
private isHighlighted(d: SettingDescriptor, highlighted: ReadonlySet<string>): boolean {
233233
if (!('key' in d) || !highlighted.has(d.key)) return false;
234-
if (this.isNaturallyVisible(d)) return true;
234+
235+
if (this.isNaturallyVisible(d)) {
236+
// A key can have several visible variants (e.g. a checkbox + select pair
237+
// differing only by `enabledWhen`) — highlight just the first so the pair
238+
// doesn't double-highlight.
239+
const firstVisible = this.category.controls.find(
240+
c => 'key' in c && c.key === d.key && this.isNaturallyVisible(c),
241+
);
242+
return firstVisible === d;
243+
}
235244

236245
// Hidden match: force-reveal only when no visible variant of the key exists
237246
return !this.category.controls.some(
@@ -294,7 +303,7 @@ export class GlSettingsDetail extends SignalWatcher(LitElement) {
294303
.checked=${masterOn && !masterDisabledByOrg}
295304
?disabled=${masterDisabledByOrg}
296305
label="Enable ${category.name}"
297-
title=${ifDefined(
306+
hint=${ifDefined(
298307
masterDisabledByOrg
299308
? 'AI features have been disabled by your GitKraken admin.'
300309
: undefined,

src/webviews/apps/settings/components/settings-nav.ts

Lines changed: 2 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { SignalWatcher } from '@lit-labs/signals';
33
import { css, html, LitElement, nothing } from 'lit';
44
import { customElement, property } from 'lit/decorators.js';
55
import type { AutolinkConfig } from '../../../../config.js';
6-
import { focusOutlineButton } from '../../shared/components/styles/lit/a11y.css.js';
6+
import { focusOutlineButton, srOnly } from '../../shared/components/styles/lit/a11y.css.js';
77
import { boxSizingBase, linkBase } from '../../shared/components/styles/lit/base.css.js';
88
import type { CheckDescriptor, SettingsCategory, SettingsGroup, SettingsSearchMatch } from '../model.js';
99
import type { SettingsState } from '../state.js';
@@ -25,6 +25,7 @@ export class GlSettingsNav extends SignalWatcher(LitElement) {
2525
static override styles = [
2626
boxSizingBase,
2727
linkBase,
28+
srOnly,
2829
css`
2930
:host {
3031
display: block;
@@ -129,18 +130,6 @@ export class GlSettingsNav extends SignalWatcher(LitElement) {
129130
.empty p {
130131
margin: 0 0 var(--gl-space-8);
131132
}
132-
133-
.sr-only {
134-
position: absolute;
135-
width: 1px;
136-
height: 1px;
137-
padding: 0;
138-
margin: -1px;
139-
overflow: hidden;
140-
white-space: nowrap;
141-
border: 0;
142-
clip: rect(0, 0, 0, 0);
143-
}
144133
`,
145134
];
146135

src/webviews/apps/shared/components/slider/slider.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -77,7 +77,7 @@ export class GlSlider extends LitElement {
7777
step=${this.step}
7878
?disabled=${this.disabled}
7979
label=${ifDefined(this.label)}
80-
aria-valuetext=${ifDefined(this.unit ? `${this.value}${this.unit}` : undefined)}
80+
.valueFormatter=${(value: number) => `${value}${this.unit}`}
8181
@input=${this.handleInput}
8282
@change=${this.handleChange}
8383
></wa-slider>

src/webviews/apps/shared/components/switch/switch.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import type WaSwitch from '@awesome.me/webawesome/dist/components/switch/switch.js';
22
import { html, LitElement, nothing } from 'lit';
33
import { customElement, property, query } from 'lit/decorators.js';
4+
import { ifDefined } from 'lit/directives/if-defined.js';
45
import { switchStyles } from './switch.css.js';
56
import '@awesome.me/webawesome/dist/components/switch/switch.js';
67
import '../shoelace-stub.js';
@@ -38,6 +39,14 @@ export class GlSwitch extends LitElement {
3839
@property({ type: String })
3940
label?: string;
4041

42+
/**
43+
* Optional description, surfaced to assistive tech via `wa-switch`'s `hint`
44+
* (which wires it to the inner control's `aria-describedby`) — host-level ARIA
45+
* never reaches the focused input. Renders visibly beneath the switch.
46+
*/
47+
@property({ type: String })
48+
hint?: string;
49+
4150
@property({ type: String, reflect: true })
4251
size: 'medium' | 'large' = 'medium';
4352

@@ -59,6 +68,7 @@ export class GlSwitch extends LitElement {
5968
exportparts="base, control, thumb, label"
6069
.checked=${this.checked}
6170
?disabled=${this.disabled}
71+
hint=${ifDefined(this.hint)}
6272
@change=${this.handleChange}
6373
><slot>${this.label ? html`<span class="sr-only">${this.label}</span>` : nothing}</slot></wa-switch
6474
>`;

0 commit comments

Comments
 (0)