Skip to content

Commit daab45f

Browse files
committed
fix(forms): improve aria attribute management
- Modified state controllers to correctly manage aria attributes when values are null or undefined. - Improved test cases for state controllers to ensure proper aria attribute handling. - Introduced custom slider defaults in tests to validate default behavior. Signed-off-by: Cory Rylan <crylan@nvidia.com>
1 parent 98a4325 commit daab45f

18 files changed

Lines changed: 189 additions & 76 deletions

‎projects/forms/src/internal/controllers/state-current.controller.test.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ describe('StateCurrentController', () => {
6464
element.current=null;
6565
element.sync();
6666

67+
expect(element._internals!.ariaCurrent).toBe(null);
6768
expect(element.matches(':state(current)')).toBe(false);
6869
});
6970

@@ -86,6 +87,17 @@ describe('StateCurrentController', () => {
8687
expect(element.matches(':state(current)')).toBe(false);
8788
});
8889

90+
it('should preserve current value on anchor aria-current',()=>{
91+
constanchor=document.createElement('a');
92+
element.append(anchor);
93+
element._internals!.states.add('anchor');
94+
95+
element.current='step';
96+
element.sync();
97+
98+
expect(anchor.getAttribute('aria-current')).toBe('step');
99+
});
100+
89101
it('should remove anchor aria-current while readonly',()=>{
90102
constanchor=document.createElement('a');
91103
element.append(anchor);

‎projects/forms/src/internal/controllers/state-current.controller.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ export class StateCurrentController<T extends Current> implements ReactiveContro
2424

2525
hostUpdated(){
2626
if(this.host.readOnly){
27+
this.host._internals!.ariaCurrent=null;
28+
toggleState(this.host._internals!,'current',false);
2729
this.#syncAnchorCurrentAttribute();
2830
return;
2931
}
@@ -35,9 +37,8 @@ export class StateCurrentController<T extends Current> implements ReactiveContro
3537
return;
3638
}
3739

38-
if(this.host.current!==null&&this.host.current!==undefined){
39-
this.host._internals!.ariaCurrent=`${this.host.current}`;
40-
}
40+
this.host._internals!.ariaCurrent=
41+
this.host.current===null||this.host.current===undefined ? null : `${this.host.current}`;
4142

4243
toggleState(this.host._internals!,'current',Boolean(this.host.current));
4344
}
@@ -51,7 +52,7 @@ export class StateCurrentController<T extends Current> implements ReactiveContro
5152

5253
if(anchor&&isCurrent){
5354
this.#anchorCurrentTarget?.removeAttribute('aria-current');
54-
anchor.setAttribute('aria-current','page');
55+
anchor.setAttribute('aria-current',this.host.current??'page');
5556
this.#anchorCurrentTarget =anchor;
5657
return;
5758
}

‎projects/forms/src/internal/controllers/state-expanded.controller.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ describe('StateExpandedController', () => {
6767
});
6868

6969
it('should leave aria-expanded unset for absent values',()=>{
70+
element.expanded=true;
71+
element.sync();
72+
7073
element.expanded=null;
7174
element.sync();
7275

‎projects/forms/src/internal/controllers/state-expanded.controller.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ export class StateExpandedController<T extends Expanded> implements ReactiveCont
1616
}
1717

1818
hostUpdated(){
19-
if(this.host.expanded!==null&&this.host.expanded!==undefined){
20-
this.host._internals!.ariaExpanded=`${this.host.expanded}`;
21-
}
19+
this.host._internals!.ariaExpanded=
20+
this.host.expanded===null||this.host.expanded===undefined ? null : `${this.host.expanded}`;
2221

2322
toggleState(this.host._internals!,'expanded',Boolean(this.host.expanded));
2423
}

‎projects/forms/src/internal/controllers/state-pressed.controller.test.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,9 @@ describe('StatePressedController', () => {
6767
});
6868

6969
it('should leave aria-pressed unset for absent values',()=>{
70+
element.pressed=true;
71+
element.sync();
72+
7073
element.pressed=undefined;
7174
element.sync();
7275

‎projects/forms/src/internal/controllers/state-pressed.controller.ts‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,8 @@ export class StatePressedController<T extends Pressed> implements ReactiveContro
1616
}
1717

1818
hostUpdated(){
19-
if(this.host.pressed!==null&&this.host.pressed!==undefined){
20-
this.host._internals!.ariaPressed=`${this.host.pressed}`;
21-
}
19+
this.host._internals!.ariaPressed=
20+
this.host.pressed===null||this.host.pressed===undefined ? null : `${this.host.pressed}`;
2221

2322
toggleState(this.host._internals!,'pressed',Boolean(this.host.pressed));
2423
}

‎projects/forms/src/internal/controllers/state-selected.controller.test.ts‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,11 @@ describe('StateSelectedController', () => {
6666

6767
expect(element._internals!.ariaSelected).toBe('false');
6868
expect(element.matches(':state(selected)')).toBe(false);
69+
70+
element.selected=null;
71+
element.sync();
72+
73+
expect(element._internals!.ariaSelected).toBe(null);
6974
});
7075

7176
it('should move selected state to anchor aria-current for anchor hosts',()=>{
@@ -87,6 +92,18 @@ describe('StateSelectedController', () => {
8792
expect(element.matches(':state(selected)')).toBe(false);
8893
});
8994

95+
it('should preserve current value on selected anchor aria-current',()=>{
96+
constanchor=document.createElement('a');
97+
element.append(anchor);
98+
element._internals!.states.add('anchor');
99+
100+
element.current='step';
101+
element.selected=true;
102+
element.sync();
103+
104+
expect(anchor.getAttribute('aria-current')).toBe('step');
105+
});
106+
90107
it('should remove anchor aria-current while readonly',()=>{
91108
constanchor=document.createElement('a');
92109
element.append(anchor);

‎projects/forms/src/internal/controllers/state-selected.controller.ts‎

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ export class StateSelectedController<T extends Selected> implements ReactiveCont
2424

2525
hostUpdated(){
2626
if(this.host.readOnly){
27+
this.host._internals!.ariaSelected=null;
28+
toggleState(this.host._internals!,'selected',false);
2729
this.#syncAnchorCurrentAttribute();
2830
return;
2931
}
@@ -35,9 +37,8 @@ export class StateSelectedController<T extends Selected> implements ReactiveCont
3537
return;
3638
}
3739

38-
if(this.host.selected!==null&&this.host.selected!==undefined){
39-
this.host._internals!.ariaSelected=`${this.host.selected}`;
40-
}
40+
this.host._internals!.ariaSelected=
41+
this.host.selected===null||this.host.selected===undefined ? null : `${this.host.selected}`;
4142

4243
toggleState(this.host._internals!,'selected',Boolean(this.host.selected));
4344
}
@@ -51,7 +52,7 @@ export class StateSelectedController<T extends Selected> implements ReactiveCont
5152

5253
if(anchor&&isCurrent){
5354
this.#anchorCurrentTarget?.removeAttribute('aria-current');
54-
anchor.setAttribute('aria-current','page');
55+
anchor.setAttribute('aria-current',this.host.current??'page');
5556
this.#anchorCurrentTarget =anchor;
5657
return;
5758
}

projects/forms/src/internal/controllers/type-native-button.controller.test.ts renamed to projects/forms/src/internal/controllers/type-submit.controller.test.ts

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -5,26 +5,26 @@ import { html } from 'lit';
55
import{afterEach,describe,expect,it,vi}from'vitest';
66
import{createFixture,removeFixture}from'@internals/testing';
77

8-
import{TypeNativeButtonController}from'./type-native-button.controller.js';
8+
import{TypeSubmitController}from'./type-submit.controller.js';
99
importtype{ButtonType}from'../../mixins/button.types.js';
1010
importtype{ReactiveController}from'./types.js';
1111

12-
classNativeButtonBehaviorControllerTestElementextendsHTMLElement{
12+
classTypeSubmitControllerTestElementextendsHTMLElement{
1313
disabled=false;
1414
name?: string;
1515
readOnly=false;
1616
value?: string;
1717
#type?: ButtonType;
1818
#controllers =newSet<ReactiveController>();
1919

20-
_nativeButtonBehaviorController=newTypeNativeButtonController(this);
20+
_typeSubmitController=newTypeSubmitController(this);
2121

2222
getform(){
2323
returnthis.closest('form');
2424
}
2525

2626
gettype(){
27-
returnthis.#type ??this._nativeButtonBehaviorController.defaultType;
27+
returnthis.#type ??this._typeSubmitController.defaultType;
2828
}
2929

3030
settype(value: ButtonType|undefined){
@@ -48,11 +48,11 @@ class NativeButtonBehaviorControllerTestElement extends HTMLElement {
4848
}
4949
}
5050

51-
if(!customElements.get('native-button-behavior-controller-test-element')){
52-
customElements.define('native-button-behavior-controller-test-element',NativeButtonBehaviorControllerTestElement);
51+
if(!customElements.get('forms-type-submit-controller-test-element')){
52+
customElements.define('forms-type-submit-controller-test-element',TypeSubmitControllerTestElement);
5353
}
5454

55-
describe('NativeButtonBehaviorController',()=>{
55+
describe('type-submit.controller',()=>{
5656
letfixture: HTMLElement;
5757

5858
afterEach(()=>{
@@ -61,10 +61,10 @@ describe('NativeButtonBehaviorController', () => {
6161

6262
it('should default to submit when associated with a form and no type is set',async()=>{
6363
fixture=awaitcreateFixture(
64-
html`<form><native-button-behavior-controller-test-element></native-button-behavior-controller-test-element></form>`
64+
html`<form><forms-type-submit-controller-test-element></forms-type-submit-controller-test-element></form>`
6565
);
66-
constelement=fixture.querySelector<NativeButtonBehaviorControllerTestElement>(
67-
'native-button-behavior-controller-test-element'
66+
constelement=fixture.querySelector<TypeSubmitControllerTestElement>(
67+
'forms-type-submit-controller-test-element'
6868
)!;
6969

7070
element.sync();
@@ -74,10 +74,10 @@ describe('NativeButtonBehaviorController', () => {
7474

7575
it('should trigger clicks from enter and space keyup events',async()=>{
7676
fixture=awaitcreateFixture(
77-
html`<native-button-behavior-controller-test-element></native-button-behavior-controller-test-element>`
77+
html`<forms-type-submit-controller-test-element></forms-type-submit-controller-test-element>`
7878
);
79-
constelement=fixture.querySelector<NativeButtonBehaviorControllerTestElement>(
80-
'native-button-behavior-controller-test-element'
79+
constelement=fixture.querySelector<TypeSubmitControllerTestElement>(
80+
'forms-type-submit-controller-test-element'
8181
)!;
8282
constclick=vi.spyOn(element,'click').mockImplementation(()=>undefined);
8383

@@ -91,11 +91,11 @@ describe('NativeButtonBehaviorController', () => {
9191

9292
it('should submit with hidden native submitter data',async()=>{
9393
fixture=awaitcreateFixture(
94-
html`<form><native-button-behavior-controller-test-element></native-button-behavior-controller-test-element></form>`
94+
html`<form><forms-type-submit-controller-test-element></forms-type-submit-controller-test-element></form>`
9595
);
9696
constform=fixture.querySelector<HTMLFormElement>('form')!;
97-
constelement=fixture.querySelector<NativeButtonBehaviorControllerTestElement>(
98-
'native-button-behavior-controller-test-element'
97+
constelement=fixture.querySelector<TypeSubmitControllerTestElement>(
98+
'forms-type-submit-controller-test-element'
9999
)!;
100100
constrequestSubmit=vi.spyOn(form,'requestSubmit').mockImplementation(()=>undefined);
101101

@@ -116,11 +116,11 @@ describe('NativeButtonBehaviorController', () => {
116116

117117
it('should reset the associated form when type is reset',async()=>{
118118
fixture=awaitcreateFixture(
119-
html`<form><native-button-behavior-controller-test-element></native-button-behavior-controller-test-element></form>`
119+
html`<form><forms-type-submit-controller-test-element></forms-type-submit-controller-test-element></form>`
120120
);
121121
constform=fixture.querySelector<HTMLFormElement>('form')!;
122-
constelement=fixture.querySelector<NativeButtonBehaviorControllerTestElement>(
123-
'native-button-behavior-controller-test-element'
122+
constelement=fixture.querySelector<TypeSubmitControllerTestElement>(
123+
'forms-type-submit-controller-test-element'
124124
)!;
125125
constreset=vi.spyOn(form,'reset').mockImplementation(()=>undefined);
126126

@@ -133,10 +133,10 @@ describe('NativeButtonBehaviorController', () => {
133133

134134
it('should remove listeners when disabled, readonly, or disconnected',async()=>{
135135
fixture=awaitcreateFixture(
136-
html`<native-button-behavior-controller-test-element></native-button-behavior-controller-test-element>`
136+
html`<forms-type-submit-controller-test-element></forms-type-submit-controller-test-element>`
137137
);
138-
constelement=fixture.querySelector<NativeButtonBehaviorControllerTestElement>(
139-
'native-button-behavior-controller-test-element'
138+
constelement=fixture.querySelector<TypeSubmitControllerTestElement>(
139+
'forms-type-submit-controller-test-element'
140140
)!;
141141
constclick=vi.spyOn(element,'click').mockImplementation(()=>undefined);
142142

projects/forms/src/internal/controllers/type-native-button.controller.ts renamed to projects/forms/src/internal/controllers/type-submit.controller.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { onKeys, stopEvent } from '../utils.js';
55
importtype{ButtonType}from'../../mixins/button.types.js';
66
importtype{ReactiveController,ReactiveElement}from'./types.js';
77

8-
typeNativeButtonBehaviorHost=ReactiveElement&{
8+
typeSubmitHost=ReactiveElement&{
99
disabled: boolean;
1010
form: HTMLFormElement|null|string;
1111
name?: string;
@@ -14,7 +14,7 @@ type NativeButtonBehaviorHost = ReactiveElement & {
1414
value?: string;
1515
};
1616

17-
exportclassTypeNativeButtonController<TextendsNativeButtonBehaviorHost>implementsReactiveController{
17+
exportclassTypeSubmitController<TextendsSubmitHost>implementsReactiveController{
1818
#defaultType: ButtonType|undefined;
1919
#submitter: HTMLButtonElement|undefined;
2020
#submitterForm: HTMLFormElement|undefined;
@@ -29,15 +29,15 @@ export class TypeNativeButtonController<T extends NativeButtonBehaviorHost> impl
2929

3030
hostUpdated(){
3131
this.#setButtonType();
32-
this.#removeNativeButtonBehavior();
32+
this.#removeSubmitBehavior();
3333
if(!this.host.readOnly&&!this.host.disabled){
34-
this.host.addEventListener('click',this.#onNativeButtonClick);
35-
this.host.addEventListener('keyup',this.#onNativeButtonKeyup);
34+
this.host.addEventListener('click',this.#onSubmitClick);
35+
this.host.addEventListener('keyup',this.#onSubmitKeyup);
3636
}
3737
}
3838

3939
hostDisconnected(){
40-
this.#removeNativeButtonBehavior();
40+
this.#removeSubmitBehavior();
4141
this.#removeSubmitter();
4242
}
4343

@@ -47,16 +47,16 @@ export class TypeNativeButtonController<T extends NativeButtonBehaviorHost> impl
4747
}
4848
}
4949

50-
#removeNativeButtonBehavior(){
51-
this.host.removeEventListener('click',this.#onNativeButtonClick);
52-
this.host.removeEventListener('keyup',this.#onNativeButtonKeyup);
50+
#removeSubmitBehavior(){
51+
this.host.removeEventListener('click',this.#onSubmitClick);
52+
this.host.removeEventListener('keyup',this.#onSubmitKeyup);
5353
}
5454

55-
#onNativeButtonKeyup=(event: KeyboardEvent)=>{
55+
#onSubmitKeyup=(event: KeyboardEvent)=>{
5656
onKeys(['Enter','Space'],event,()=>this.host.click());
5757
};
5858

59-
#onNativeButtonClick=(event: Event)=>{
59+
#onSubmitClick=(event: Event)=>{
6060
if(this.host.disabled){
6161
stopEvent(event);
6262
return;

0 commit comments

Comments
 (0)