-
Notifications
You must be signed in to change notification settings - Fork 372
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: Different aria-labels for un-/selected values for radio-buttons #19015
base: develop
Are you sure you want to change the base?
Conversation
4 flaky tests on run #44173 ↗︎Details:
|
Test | Artifacts | |
---|---|---|
Apparel - checkout as guest > Desktop > should perform checkout as guest, create an account and verify guest data |
Test Replay
Screenshots
Video
|
ssr/pages.core-e2e.cy.ts • 3 flaky tests • SSR
Test | Artifacts | |
---|---|---|
SSR > should render homepage |
Test Replay
Screenshots
Video
|
|
SSR > should render PLP |
Test Replay
Screenshots
Video
|
|
SSR > should render PDP |
Test Replay
Screenshots
Video
|
Review all test suite changes for PR #19015 ↗︎
this.translation | ||
.translate( | ||
'configurator.a11y.selectedValueOfAttributeFullWithPrice', | ||
{ | ||
value: value.valueDisplay, | ||
attribute: attribute.label, | ||
price: value.valuePrice.formattedValue, | ||
} | ||
) | ||
.translate(translationKey, { | ||
value: value.valueDisplay, | ||
attribute: attribute.label, | ||
price: value.valuePrice.formattedValue, | ||
}) |
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.
@steinsebastian can you do me a favor to refactor the method,
- so there is only one call to the translation service.
- that multi level if can be replacded by a simple if - else if - else
psydo code
if(totalprice != 0) {
key = ...
params = ...
} else if (valueprice != 0){
key = ...
params = ...
} else{
key = ...
params = ...
}
transService.translate.....
return
i believe this makes it easier to understand what the method does and that there are exactly 3 paths.
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.
No description provided.