feat(a11y): add a11y selected label improvements to checkout Address/PaymentMethod components #19381
Conversation
…DeliveryAddress/CheckoutPaymentMethod components
spartacus
|
||||||||||||||||||||||||||
| Project |
spartacus
|
| Run status |
|
| Run duration | 11m 58s |
| Commit |
|
| Committer | Uros Lates |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
4
|
Pending
|
2
|
Skipped
|
0
|
Passing
|
125
|
Upgrade your plan to view test results. | |
| @@ -329,4 +335,12 @@ export class CheckoutDeliveryAddressComponent implements OnInit { | |||
| protected shouldUseAddressSavedInCart(): boolean { | |||
| return !!this.checkoutConfigService?.shouldUseAddressSavedInCart(); | |||
| } | |||
|
|
|||
| private getCardRole(isCardSelected: boolean): 'button' | 'region' { | |||
There was a problem hiding this comment.
Nice use of the feature flags here! Lets make this method protected instead of private so that lib users can still access it if they have a need to see/modify it without cloning the repo.
There was a problem hiding this comment.
Lets also add a unit test assertion to check that the role is set correctly since there is some logic here.
There was a problem hiding this comment.
👍 Addressed both of these in latest commit!
...e-libs/checkout/base/components/checkout-payment-method/checkout-payment-method.component.ts
Show resolved
Hide resolved
…method for checkout-delivery-address.component.ts
…method for checkout-payment-method.component.ts
| ).role).toEqual('region'); | ||
| }); | ||
|
|
||
| it('should be set to "region" when feature flag is disabled', () => { |
There was a problem hiding this comment.
We don't really need to test for the feature flags because they will be removed anyway next major and we can always assume they are active for unit tests. The main thing to test here was that isSelected sets the role correctly. Is it possible to remove any reference to feature flags here?
| @@ -500,4 +562,11 @@ describe('CheckoutDeliveryAddressComponent', () => { | |||
| expect(getSpinner()).toBeFalsy(); | |||
| }); | |||
| }); | |||
|
|
|||
| // describe('getCardRole()', () => { | |||
| expect(component['getCardRole']).toHaveBeenCalledWith(true); | ||
| }); | ||
|
|
||
| it('should be set to "region" when feature flag is disabled', () => { |
There was a problem hiding this comment.
Same as below about testing feature flags.
| it('should be set to "region" for selected addresses when feature flag is enabled', () => { | ||
| spyOn(featureConfig, 'isEnabled').and.returnValue(true); | ||
| expect(component.getCardContent( | ||
| // isSelected = true! |
There was a problem hiding this comment.
👍 Addressed all test related comments in latest commit.
No description provided.