-
Notifications
You must be signed in to change notification settings - Fork 393
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.
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
fix: (CXSPA-9005) fix ng-select options issue - <y of x> is conveyed twice #19844
base: develop
Are you sure you want to change the base?
Conversation
Merge Checks Failed
|
spartacus Run #46667
Run Properties:
|
Project |
spartacus
|
Branch Review |
feature/CXSPA-9005
|
Run status |
Passed #46667
|
Run duration | 04m 07s |
Commit |
170e748acf ℹ️: Merge 0f3aaf327e7e71f0d705b254688c3ac6079df0de into e7b7e653ec7e07b35787b6402c39...
|
Committer | Uros Lates |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
4
|
Pending |
2
|
Skipped |
0
|
Passing |
125
|
View all changes introduced in this branch ↗︎ |
projects/storefrontlib/shared/components/ng-select-a11y/ng-select-a11y.directive.ts
Show resolved
Hide resolved
} | ||
); | ||
}); | ||
options.forEach((option: HTMLOptionElement) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we need a feature toggle here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks Stan for the heads up & catching this one!
I've reiterated over with Caine and code is updated now.
Changed the implementation - setting a11yNgSelectOptionsCount to false (per default) will cause ng-select options ariaLabels to working as expected. Marked all relevant directive methods as depricated. Hopefully the depricated code (including the flag) would be removed in next major release (undoing part of old directive which is now introducing issue) - making everything work normal. |
a11yNgSelectOptionsCount: true, | ||
/** | ||
* Defaults to false cause ng-select options ariaLabels are working as expected | ||
* since Spartacus 2211.33 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets also reference the issue CX-9005 here and in feature-toggles.ts
so anybody curious can reference the discussion about this flag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated comment with reference to the related issue.
Closes https://jira.tools.sap/browse/CXSPA-9005