-
Notifications
You must be signed in to change notification settings - Fork 386
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-8034) remove aria label from title dropdown #19325
fix: (CXSPA-8034) remove aria label from title dropdown #19325
Conversation
StanislavSukhanov
commented
Oct 3, 2024
- closes https://jira.tools.sap/browse/CXSPA-8034
spartacus Run #45449
Run Properties:
|
Project |
spartacus
|
Branch Review |
feature/CXSPA-8034-user-register-remove-aria-label-from-title-dropdown
|
Run status |
Passed #45449
|
Run duration | 03m 49s |
Commit |
56eb409047 ℹ️: Merge 9d10c06d646217b65bdf9ea08e2a72196dffd734 into c42053bf68bd6fcec3c819cd2f53...
|
Committer | Stanislav Sukhanov |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
3
|
Pending |
2
|
Skipped |
0
|
Passing |
125
|
View all changes introduced in this branch ↗︎ |
@StanislavSukhanov for some reason the combobox now gets identified as a "Link". Please compare this with another optional combobox like the one on the Shipping Address step during Checkout. Let's please bring back the item count for VoiceOver and rework the code to make JAWS narrate the element as "edit combo". |
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.
Pls see previously attached screenshots
Here's my JAWS output. It's not in the history, but I can hear the word "link" after tabbing to the input. |
aede108
fbba011
to
aede108
Compare
Hi @Pio-Bar. Thanks for the advice, I did suspected that too. I've put back the directive. The items count is present again in the Voice Over. |
Merge Checks Failed
|
@@ -30,7 +30,6 @@ | |||
"confirmNewPassword": "Confirm New Password", | |||
"resetPassword": "Reset Password", | |||
"createAccount": "Create an account", | |||
"title": "Title", |
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'm not sure about removing this translation key as it would constitute a breaking change. Can you confirm if the key has only been introduced to set the aria-label on ng-select? And nowhere else in the past?
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.
Hi @Zeyber! I have checked in the repository and couldn't find such combination other than in a place it was removed from.
Please let me know if I need to revert this change. Thanks a lot 👍
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.
@Zeyber reverted, fixed.
aede108
to
92714c2
Compare
@developpeurweb It is fixed. Could you please have a look on that again? Thank you |
Merge Checks Failed
|
Merge Checks Failed
|
* add ngSelectA11y directive back to the select component * set searchable property of title select to true.
* revert deletion of a translation key
* fix Lighthouse pipeline; * closes https://jira.tools.sap/browse/CXSPA-8156
aa39aeb
ae55c38
to
aa39aeb
Compare
* remove searchable attribute; * closes https://jira.tools.sap/browse/CXSPA-8156