Skip to content

fix(segment): use a single roving tab stop #31529

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

Open
minwookshin wants to merge 1 commit into
base: main
Choose a base branch
from
+137 −1
Open

fix(segment): use a single roving tab stop #31529

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions core/src/components.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2959,6 +2959,10 @@ export namespace Components {
* @default true
*/
"swipeGesture": boolean;
/**
* Update the tab stop when a segment button changes.
*/
"updateTabindex": () => Promise<void>;
/**
* the value of the segment.
*/
Expand All @@ -2983,6 +2987,7 @@ export namespace Components {
* The mode determines which platform styles to use.
*/
"mode"?: "ios" | "md";
"setButtonTabindex": (value: number) => Promise<void>;
"setFocus": () => Promise<void>;
/**
* The type of the button.
Expand Down
20 changes: 20 additions & 0 deletions core/src/components/segment-button/segment-button.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
@Element() el!: HTMLElement;

@State() checked = false;
@State() private buttonTabindex = 0;

/**
* The `id` of the segment content.
Expand All @@ -46,6 +47,10 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
* If `true`, the user cannot interact with the segment button.
*/
@Prop({ mutable: true }) disabled = false;
@Watch('disabled')
protected disabledChanged() {
this.segmentEl?.updateTabindex();
}

/**
* Set the layout of the text and icon in the segment.
Expand All @@ -64,6 +69,7 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
@Watch('value')
valueChanged() {
this.updateState();
this.segmentEl?.updateTabindex();
}

connectedCallback() {
Expand All @@ -72,6 +78,7 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
this.updateState();
addEventListener(segmentEl, 'ionSelect', this.updateState);
addEventListener(segmentEl, 'ionStyle', this.updateStyle);
segmentEl.updateTabindex();
}
}

Expand Down Expand Up @@ -122,6 +129,18 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
}
}

/**
* @internal
*/
@Method()
async setButtonTabindex(value: number) {
this.buttonTabindex = value;
// Keep the native tab stop in sync before the next keyboard event.
if (this.nativeEl) {
this.nativeEl.tabIndex = value;
}
}

private get hasLabel() {
return !!this.el.querySelector('ion-label');
}
Expand Down Expand Up @@ -192,6 +211,7 @@ export class SegmentButton implements ComponentInterface, ButtonInterface {
class="button-native"
part="native"
disabled={disabled}
tabindex={this.buttonTabindex}
{...this.inheritedAttributes}
>
<span class="button-inner">
Expand Down
31 changes: 30 additions & 1 deletion core/src/components/segment/segment.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import type { ComponentInterface, EventEmitter } from '@stencil/core';
import { Component, Element, Event, Host, Listen, Prop, State, Watch, h, writeTask } from '@stencil/core';
import { Component, Element, Event, Host, Listen, Method, Prop, State, Watch, h, writeTask } from '@stencil/core';
import type { Gesture, GestureDetail } from '@utils/gesture';
import { raf } from '@utils/helpers';
import { isRTL } from '@utils/rtl';
Expand All @@ -24,6 +24,7 @@ import type { SegmentChangeEventDetail, SegmentValue } from './segment-interface
})
export class Segment implements ComponentInterface {
private gesture?: Gesture;
private focusedButton?: HTMLIonSegmentButtonElement;

// Value before the segment is dragged
private valueBeforeGesture?: SegmentValue;
Expand Down Expand Up @@ -117,6 +118,7 @@ export class Segment implements ComponentInterface {
* Used by `ion-segment-button` to determine if the button should be checked.
*/
this.ionSelect.emit({ value });
this.updateTabindex();

// The scroll listener should handle scrolling the active button into view as needed
if (!this.segmentViewEl) {
Expand Down Expand Up @@ -167,6 +169,31 @@ export class Segment implements ComponentInterface {
}
}

/**
* Update the tab stop when a segment button changes.
* @internal
*/
@Method()
async updateTabindex() {
const buttons = this.getButtons();
const enabled = buttons.filter((button) => !button.disabled);
const focused = enabled.find((button) => button === this.focusedButton);
const selected = enabled.find((button) => button.value === this.value);
const focusable = focused ?? selected ?? enabled[0];

for (const button of buttons) {
button.setButtonTabindex(button === focusable ? 0 : -1);
}
}

@Listen('focusin')
@Listen('focusout')
protected focusChanged(ev: FocusEvent) {
const target = ev.type === 'focusin' ? ev.target : ev.relatedTarget;
this.focusedButton = this.getButtons().find((button) => button === target);
this.updateTabindex();
}

private gestureChanged() {
if (this.gesture) {
this.gesture.enable(!this.scrollable && !this.disabled && this.swipeGesture);
Expand All @@ -180,6 +207,7 @@ export class Segment implements ComponentInterface {
}

disconnectedCallback() {
this.focusedButton = undefined;
this.segmentViewEl = null;
}

Expand All @@ -191,6 +219,7 @@ export class Segment implements ComponentInterface {
this.segmentViewEl = this.getSegmentView();

this.setCheckedClasses();
this.updateTabindex();

/**
* We need to wait for the buttons to all be rendered
Expand Down
82 changes: 82 additions & 0 deletions core/src/components/segment/test/a11y/segment.e2e.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,88 @@ configs().forEach(({ title, config }) => {
expect(results.violations).toEqual([]);
});

test('should enter the selected button and leave the segment with Tab', async ({ page, pageUtils }) => {
await page.setContent(
`<button id="before">Before</button>
<ion-segment value="second">
<ion-segment-button value="first">First</ion-segment-button>
<ion-segment-button value="second">Second</ion-segment-button>
<ion-segment-button value="third">Third</ion-segment-button>
</ion-segment>
<button id="after">After</button>`,
config
);
const buttons = page.locator('ion-segment-button');
await page.locator('#before').focus();
await pageUtils.pressKeys('Tab');
await expect(buttons.nth(1)).toBeFocused();
await pageUtils.pressKeys('Tab');
await expect(page.locator('#after')).toBeFocused();
await pageUtils.pressKeys('Shift+Tab');
await expect(buttons.nth(1)).toBeFocused();

// Manual activation moves focus without changing the selected button.
await page.keyboard.press('Home');
await expect(buttons.nth(0)).toBeFocused();
await expect(buttons.nth(1).locator('button')).toHaveAttribute('aria-selected', 'true');
await pageUtils.pressKeys('Tab');
await expect(page.locator('#after')).toBeFocused();
await pageUtils.pressKeys('Shift+Tab');
await expect(buttons.nth(1)).toBeFocused();
await page.keyboard.press('End');
await expect(buttons.nth(2)).toBeFocused();
await pageUtils.pressKeys('Shift+Tab');
await expect(page.locator('#before')).toBeFocused();
});

test('should keep one enabled tab stop when buttons change', async ({ page }) => {
await page.setContent(
`<ion-segment value="second">
<ion-segment-button value="first" disabled>First</ion-segment-button>
<ion-segment-button value="second">Second</ion-segment-button>
<ion-segment-button value="third">Third</ion-segment-button>
</ion-segment>`,
config
);
const tabStop = page.locator('ion-segment-button button[tabindex="0"]');
await expect(tabStop).toHaveCount(1);
await expect(tabStop).toHaveAccessibleName('Second');
await page.locator('ion-segment-button[value="second"]').evaluate((button: HTMLIonSegmentButtonElement) => {
button.disabled = true;
});
await expect(tabStop).toHaveCount(1);
await expect(tabStop).toHaveAccessibleName('Third');
await page.locator('ion-segment-button[value="first"]').evaluate((button: HTMLIonSegmentButtonElement) => {
button.disabled = false;
});
await expect(tabStop).toHaveCount(1);
await expect(tabStop).toHaveAccessibleName('First');
await page.locator('ion-segment').evaluate((segment: HTMLIonSegmentElement) => {
segment.value = 'third';
});
await expect(tabStop).toHaveAccessibleName('Third');
await page.locator('ion-segment-button[value="third"]').evaluate((button) => button.remove());
await expect(tabStop).toHaveCount(1);
await expect(tabStop).toHaveAccessibleName('First');
await page.locator('ion-segment').evaluate((segment: HTMLIonSegmentElement) => {
segment.value = 'added';
const button = document.createElement('ion-segment-button');
button.value = 'added';
button.textContent = 'Added';
segment.append(button);
});
await expect(tabStop).toHaveCount(1);
await expect(tabStop).toHaveAccessibleName('Added');
await page.locator('ion-segment-button[value="first"] button').focus();
await expect(tabStop).toHaveAccessibleName('First');
await page.locator('ion-segment').evaluate((segment) => {
segment.remove();
document.body.append(segment);
});
await expect(tabStop).toHaveCount(1);
await expect(tabStop).toHaveAccessibleName('Added');
});

test('segment buttons should be keyboard navigable', async ({ page, pageUtils }) => {
const isRTL = config.direction === 'rtl';
const nextKey = isRTL ? 'ArrowLeft' : 'ArrowRight';
Expand Down
Loading