feat: Add vocalization of dropdown item count#21286
feat: Add vocalization of dropdown item count#21286BenjaminEmiliani merged 12 commits intodevelopfrom
Conversation
spartacus
|
||||||||||||||||||||||||||||
| Project |
spartacus
|
| Branch Review |
fix/CXSPA-12456-vocalize_list_item_count
|
| Run status |
|
| Run duration | 04m 15s |
| Commit |
|
| Committer | Benjamin Emiliani |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
3
|
|
|
0
|
|
|
0
|
|
|
101
|
| View all changes introduced in this branch ↗︎ | |
Merge Checks Failed |
Merge Checks Failed |
Merge Checks Failed |
| ) { | ||
| useFeatureStyles('a11yNgSelectUnicodeCarets'); | ||
| if (this.featureConfigService.isEnabled('a11yVocalizeDropdownItemCount')) { | ||
| effect(() => { |
There was a problem hiding this comment.
Please add a new method and move this logic into it, and call it in the constructor.
It will allow overraidabilty
| if (!itemCountSpan) { | ||
| itemCountSpan = this.renderer.createElement('span'); | ||
| this.renderer.addClass(itemCountSpan, 'cx-ng-select-count'); | ||
| this.renderer.addClass(itemCountSpan, 'cx-visually-hidden'); | ||
| this.renderer.setAttribute(itemCountSpan, ARIA_HIDDEN, 'true'); | ||
| this.renderer.appendChild( | ||
| this.elementRef.nativeElement, | ||
| itemCountSpan | ||
| ); | ||
| const countId = | ||
| (this.elementRef.nativeElement.id || 'ng-select') + '-count'; | ||
| this.renderer.setAttribute(itemCountSpan, 'id', countId); | ||
| const inputCombobox = | ||
| this.elementRef.nativeElement.querySelector( | ||
| '[role="combobox"]' | ||
| ); | ||
| if (inputCombobox) { | ||
| this.renderer.setAttribute( | ||
| inputCombobox, | ||
| 'aria-describedby', | ||
| countId | ||
| ); | ||
| } | ||
| this.destroyRef.onDestroy(() => itemCountSpan.remove()); | ||
| } |
There was a problem hiding this comment.
Imo woth to separate into another method:
protected createItemCountSpan() {
const itemCountSpan = this.renderer.createElement('span');
this.renderer.addClass(itemCountSpan, 'cx-ng-select-count');
this.renderer.addClass(itemCountSpan, 'cx-visually-hidden');
this.renderer.setAttribute(itemCountSpan, ARIA_HIDDEN, 'true');
this.renderer.appendChild(this.elementRef.nativeElement, itemCountSpan);
const countId =
(this.elementRef.nativeElement.id || 'ng-select') + '-count';
this.renderer.setAttribute(itemCountSpan, 'id', countId);
const inputCombobox =
this.elementRef.nativeElement.querySelector('[role="combobox"]');
if (inputCombobox) {
this.renderer.setAttribute(inputCombobox, 'aria-describedby', countId);
}
this.destroyRef.onDestroy(() => itemCountSpan.remove());
return itemCountSpan;
}| let itemCountSpan = this.elementRef.nativeElement.querySelector( | ||
| '.cx-ng-select-count' | ||
| ); |
There was a problem hiding this comment.
Then we can simplify
| let itemCountSpan = this.elementRef.nativeElement.querySelector( | |
| '.cx-ng-select-count' | |
| ); | |
| const itemCountSpan = this.elementRef.nativeElement.querySelector( | |
| '.cx-ng-select-count' | |
| ) ?? createItemCountSpan(); |
| "ngSelectDropdownCount": "{{count}} option available", | ||
| "ngSelectDropdownCount_other": "{{count}} options available" |
There was a problem hiding this comment.
ngSelectDropdownOptionsList
@uroslates I can see you introduced it year ago and Ben would like to continue this semantic.
Is it some new customary to include the library name in the translation key? Is there a reason for that?
| "ngSelectDropdownCount": "{{count}} option available", | |
| "ngSelectDropdownCount_other": "{{count}} options available" | |
| "dropdownCount": "{{count}} option available", | |
| "dropdownCount_other": "{{count}} options available" |
There was a problem hiding this comment.
The only reason I can think of is clarity
There was a problem hiding this comment.
Renamed to dropdownItemCount
There was a problem hiding this comment.
Yes the reason was clarity because at that time the lib did not support customisation for these kind of customization. Later they introduced the config for controlling this. I think its good that you've renamed it.
Merge Checks Failed |
| private elementRef: ElementRef | ||
| ) { | ||
| useFeatureStyles('a11yNgSelectUnicodeCarets'); | ||
| this.vocalizeItemCount(); |
There was a problem hiding this comment.
Would prefer ratcher
| this.vocalizeItemCount(); | |
| if (this.featureConfigService.isEnabled('a11yVocalizeDropdownItemCount')) { | |
| this.vocalizeItemCount(); | |
| } |
what do you think?
Tested and validated on iOS 26.4 - iPhone 17. Behaviour is almost identical to what is observed on mac Voice Over.