Skip to content

Commit 4533c2e

Browse files
MariaIDinevamikerodonnell89
MariaIDineva
and
mikerodonnell89
authored
fix(core): add some a11y improvements to segmented button (#13505)
* fix(core): add some a11y improvements to segmented button * fix(core): move logic for aria-selected to base button, add unit tests * test(platform): update unit test * fix(core): add translations for aria-roledescription attributes * fix(core): fix format errors * fix(core): fix format errors again * fix(core): fix disabled selected button styles * fix(core): fix circular dependency error, update examples * fix(core): update aria attributes, fix css and unit tests * fix(core): resolve comments * fix(core): remove redundant forwardRef, add some unit tests * fix(core): make id in FocusableListItemFocusedEvent optional * fix(core): order of properties in itemFocused event --------- Co-authored-by: [email protected] <[email protected]>
1 parent 5f00e6a commit 4533c2e

File tree

85 files changed

+486
-79
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

85 files changed

+486
-79
lines changed

‎libs/cdk/utils/directives/focusable-list/focusable-list.directive.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ import { ScrollPosition, scrollIntoView } from './scroll';
4141
export interface FocusableListItemFocusedEvent {
4242
index: number;
4343
total: number;
44+
id?: string | null;
4445
}
4546

4647
export interface FocusableListKeydownEvent {
@@ -364,7 +365,8 @@ export class FocusableListDirective implements OnChanges, AfterViewInit, OnDestr
364365
this._gridItemFocused$.next(directiveItem._position!);
365366
}
366367

367-
this.itemFocused.next({ index, total: items.length });
368+
const id = getItemElement(item)?.id ?? null;
369+
this.itemFocused.next({ index, total: items.length, id });
368370
this._focusableItems.forEach((i) => i.setTabbable(i === directiveItem));
369371
this._keyManager?.setActiveItem(index);
370372
})

‎libs/core/button/base-button.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,11 @@ export class BaseButton implements HasElementRef {
3737
@Input({ transform: booleanAttribute })
3838
toggled: BooleanInput;
3939

40+
/** Whether button is selected. */
41+
@HostBinding('attr.aria-selected')
42+
@Input({ transform: booleanAttribute })
43+
selected: BooleanInput;
44+
4045
/**
4146
* Native type of button element
4247
*/

‎libs/core/button/button.component.spec.ts‎

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -31,25 +31,22 @@ export class DisabledTestComponent {}
3131
export class AriaDisabledTestComponent {}
3232

3333
describe('ButtonComponent', () => {
34-
let fixture: ComponentFixture<TestComponent>, debugElement: DebugElement;
35-
let component, componentInstance: ButtonComponent;
34+
let fixture: ComponentFixture<ButtonComponent>, componentInstance: ButtonComponent;
3635

3736
beforeEach(waitForAsync(() => {
3837
TestBed.configureTestingModule({
39-
imports: [ButtonModule, TestComponent]
38+
imports: [ButtonComponent]
4039
}).compileComponents();
4140
}));
4241

4342
beforeEach(() => {
44-
fixture = TestBed.createComponent(TestComponent);
45-
debugElement = fixture.debugElement;
43+
fixture = TestBed.createComponent(ButtonComponent);
4644
fixture.detectChanges();
47-
component = debugElement.query(By.directive(ButtonComponent));
48-
componentInstance = component.injector.get(ButtonComponent);
45+
componentInstance = fixture.componentInstance;
4946
});
5047

51-
it('should create TestComponent', () => {
52-
expect(component).toBeTruthy();
48+
it('should create ButtonComponent', () => {
49+
expect(componentInstance).toBeTruthy();
5350
});
5451

5552
it('should add appropriate classes', () => {
@@ -67,6 +64,16 @@ describe('ButtonComponent', () => {
6764
componentInstance.ngOnInit();
6865
expect(componentInstance.buildComponentCssClass).toHaveBeenCalled();
6966
});
67+
68+
it('should set a default id', () => {
69+
expect(componentInstance.id()).toBe('fd-button-4');
70+
});
71+
72+
it('should set a custom id if such is provided', () => {
73+
fixture.componentRef.setInput('id', 'custom-id');
74+
fixture.detectChanges();
75+
expect(componentInstance.id()).toBe('custom-id');
76+
});
7077
});
7178

7279
describe('ButtonComponent – Disabled', () => {

‎libs/core/button/button.component.ts‎

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import {
44
computed,
55
ElementRef,
66
HostListener,
7+
input,
78
Input,
89
OnChanges,
910
OnDestroy,
@@ -18,6 +19,8 @@ import { BaseButton } from './base-button';
1819
import { IconComponent } from '@fundamental-ngx/core/icon';
1920
import { FD_BUTTON_COMPONENT } from './tokens';
2021

22+
let buttonId = 0;
23+
2124
/**
2225
* Button directive, used to enhance standard HTML buttons.
2326
*
@@ -40,7 +43,8 @@ import { FD_BUTTON_COMPONENT } from './tokens';
4043
'[attr.type]': 'type',
4144
'[attr.disabled]': 'disabled || null',
4245
'[attr.aria-label]': 'buttonArialabel()',
43-
'[attr.aria-description]': 'buttonAriaDescription()'
46+
'[attr.aria-description]': 'buttonAriaDescription()',
47+
'[attr.id]': 'id()'
4448
},
4549
providers: [
4650
contentDensityObserverProviders(),
@@ -59,6 +63,9 @@ export class ButtonComponent
5963
@Input()
6064
class = '';
6165

66+
/** Button ID - default value is provided if not set */
67+
id = input(`fd-button-${++buttonId}`);
68+
6269
/** @hidden */
6370
specialButtonType: Array<string> = ['emphasized', 'positive', 'negative', 'attention'];
6471

‎libs/core/segmented-button/segmented-button.component.scss‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1 +1,11 @@
11
@import 'fundamental-styles/dist/segmented-button.css';
2+
3+
.fd-button.is-disabled.is-selected,
4+
.fd-button.is-disabled[aria-selected='true'],
5+
.fd-button:disabled.is-selected,
6+
.fd-button:disabled[aria-selected='true'],
7+
.fd-button[aria-disabled='true'].is-selected,
8+
.fd-button[aria-disabled='true'][aria-selected='true'] {
9+
--fdButtonBorderColor: var(--sapButton_Selected_BorderColor);
10+
--fdButtonBackgroundColor: var(--sapButton_Selected_Background);
11+
}

‎libs/core/segmented-button/segmented-button.component.spec.ts‎

Lines changed: 42 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@ import { runValueAccessorTests } from 'ngx-cva-test-suite';
66
import { SegmentedButtonComponent } from './segmented-button.component';
77
import { SegmentedButtonModule } from './segmented-button.module';
88

9-
const isSelectedClass = 'fd-button--toggled';
10-
119
@Component({
1210
selector: 'fd-test-component',
1311
template: `
@@ -50,16 +48,46 @@ describe('SegmentedButtonComponent', () => {
5048
expect(component).toBeTruthy();
5149
});
5250

51+
it('should set segmented button group classes and attributes correctly', () => {
52+
const segmentedButtonElement = fixture.debugElement.nativeElement.querySelector('fd-segmented-button');
53+
expect(segmentedButtonElement.classList).toContain('fd-segmented-button');
54+
expect(segmentedButtonElement.classList).not.toContain('fd-segmented-button--vertical');
55+
56+
expect(segmentedButtonElement.getAttribute('role')).toBe('listbox');
57+
expect(segmentedButtonElement.getAttribute('aria-multiselectable')).toBe('false');
58+
expect(segmentedButtonElement.getAttribute('aria-orientation')).toBe('horizontal');
59+
expect(segmentedButtonElement.getAttribute('aria-roledescription')).toBe('Segmented Button Group');
60+
});
61+
62+
it('should set button attributes correctly', () => {
63+
expect(component.firstButton.nativeElement.getAttribute('role')).toBe('option');
64+
expect(component.firstButton.nativeElement.getAttribute('aria-roledescription')).toBe('Segmented Button');
65+
expect(component.firstButton.nativeElement.getAttribute('aria-posinset')).toBe('1');
66+
expect(component.firstButton.nativeElement.getAttribute('aria-setsize')).toBe('3');
67+
68+
expect(component.secondButton.elementRef.nativeElement.getAttribute('role')).toBe('option');
69+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-roledescription')).toBe(
70+
'Segmented Button'
71+
);
72+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-posinset')).toBe('2');
73+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-setsize')).toBe('3');
74+
75+
expect(component.thirdButton.nativeElement.getAttribute('role')).toBe('option');
76+
expect(component.thirdButton.nativeElement.getAttribute('aria-roledescription')).toBe('Segmented Button');
77+
expect(component.thirdButton.nativeElement.getAttribute('aria-posinset')).toBe('3');
78+
expect(component.thirdButton.nativeElement.getAttribute('aria-setsize')).toBe('3');
79+
});
80+
5381
// Default Example
5482
it('should correctly select and deselect single value in non-toggle mode', () => {
5583
component.segmentedButton.writeValue('second');
5684
fixture.detectChanges();
57-
expect(component.secondButton.elementRef.nativeElement.classList.contains(isSelectedClass)).toBe(true);
85+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-selected')).toBe('true');
5886

5987
component.segmentedButton.writeValue('first');
6088
fixture.detectChanges();
61-
expect(component.firstButton.nativeElement.classList.contains(isSelectedClass)).toBe(true);
62-
expect(component.secondButton.elementRef.nativeElement.classList.contains(isSelectedClass)).toBe(false);
89+
expect(component.firstButton.nativeElement.getAttribute('aria-selected')).toBe('true');
90+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-selected')).toBe('false');
6391
});
6492

6593
// Toggle Example
@@ -69,31 +97,31 @@ describe('SegmentedButtonComponent', () => {
6997

7098
component.firstButton.nativeElement.dispatchEvent(new MouseEvent('click'));
7199
fixture.detectChanges();
72-
expect(component.firstButton.nativeElement.classList.contains(isSelectedClass)).toBe(true);
100+
expect(component.firstButton.nativeElement.getAttribute('aria-selected')).toBe('true');
73101

74102
component.secondButton.elementRef.nativeElement.dispatchEvent(new MouseEvent('click'));
75103
fixture.detectChanges();
76-
expect(component.secondButton.elementRef.nativeElement.classList.contains(isSelectedClass)).toBe(true);
104+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-selected')).toBe('true');
77105

78106
component.thirdButton.nativeElement.dispatchEvent(new MouseEvent('click'));
79107
fixture.detectChanges();
80-
expect(component.thirdButton.nativeElement.classList.contains(isSelectedClass)).toBe(true);
108+
expect(component.thirdButton.nativeElement.getAttribute('aria-selected')).toBe('true');
81109

82110
// Deselect
83111
component.firstButton.nativeElement.dispatchEvent(new MouseEvent('click'));
84112
fixture.detectChanges();
85-
expect(component.firstButton.nativeElement.classList.contains(isSelectedClass)).toBe(false);
113+
expect(component.firstButton.nativeElement.getAttribute('aria-selected')).toBe('false');
86114
});
87115

88116
// Form Example
89117
it('should update form value correctly', () => {
90118
component.segmentedButton.writeValue('first');
91119
fixture.detectChanges();
92-
expect(component.firstButton.nativeElement.classList.contains(isSelectedClass)).toBe(true);
120+
expect(component.firstButton.nativeElement.getAttribute('aria-selected')).toBe('true');
93121

94122
component.segmentedButton.writeValue('second');
95123
fixture.detectChanges();
96-
expect(component.secondButton.elementRef.nativeElement.classList.contains(isSelectedClass)).toBe(true);
124+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-selected')).toBe('true');
97125
});
98126

99127
// Disabled State Check
@@ -120,15 +148,15 @@ describe('SegmentedButtonComponent', () => {
120148

121149
component.firstButton.nativeElement.dispatchEvent(new MouseEvent('click'));
122150
fixture.detectChanges();
123-
expect(component.firstButton.nativeElement.classList.contains(isSelectedClass)).toBe(true);
151+
expect(component.firstButton.nativeElement.getAttribute('aria-selected')).toBe('true');
124152

125153
component.secondButton.elementRef.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: 'Enter' }));
126154
fixture.detectChanges();
127-
expect(component.secondButton.elementRef.nativeElement.classList.contains(isSelectedClass)).toBe(true);
155+
expect(component.secondButton.elementRef.nativeElement.getAttribute('aria-selected')).toBe('true');
128156

129157
component.thirdButton.nativeElement.dispatchEvent(new KeyboardEvent('keydown', { key: ' ' }));
130158
fixture.detectChanges();
131-
expect(component.thirdButton.nativeElement.classList.contains(isSelectedClass)).toBe(true);
159+
expect(component.thirdButton.nativeElement.getAttribute('aria-selected')).toBe('true');
132160
});
133161
});
134162

0 commit comments

Comments
 (0)