SAP / spartacus

Spartacus is a lean, Angular-based JavaScript storefront for SAP Commerce Cloud that communicates exclusively through the Commerce REST API.
Apache License 2.0
726 stars 372 forks source link

fix: (CXSPA-7616) - set color for toolbar buttons #18980

Closed Reid-McNaughton closed 1 week ago

Reid-McNaughton commented 2 weeks ago

closes: CXSPA-7616

The foreground colors used in the buttons in the floating toolbar are aligned with the colours used for the buttons in this component: image

Behaviour with a11yImproveContrast enabled (without the changes in this PR): image

Expected behaviour: image

We either need to apply the changes in this PR or alternatively we could keep this unexpected color change (which does not look bad to me) and make other changes to align with the colors used on secondary buttons (adding a colour change on hover for instance).

cypress[bot] commented 2 weeks ago

4 flaky tests on run #44162 ↗︎

0 119 2 0 Flakiness 4

Details:

Merge a8aa6fa9534abd249cab0cc5c69f0c167064b5e3 into 75e8ca4045bb4d936a77d5395b50...
Project: spartacus Commit: 1a1b0bcb86 ℹ️
Status: Passed Duration: 04:03 💡
Started: Jun 26, 2024 9:11 PM Ended: Jun 26, 2024 9:15 PM
Flakiness  b2b/regression/checkout/b2b-credit-card-checkout-flow.core-e2e.cy.ts • 1 flaky test • B2B View Output Video
Test Artifacts
B2B - Credit Card Checkout flow > should checkout using a credit card Test Replay Screenshots Video
Flakiness  ssr/pages.core-e2e.cy.ts • 3 flaky tests • SSR View Output Video
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 #18980 ↗︎

sam-garland commented 2 weeks ago

This restores the old behaviour.

sam-garland commented 2 weeks ago

@RadhepS - Would you be able to review this change?

It preserves past behaviour and maximises contrast in common scenarios. I don't actually mind the unintended colour change introduced by a11yImproveContrast, but we either need to preserve the old behaviour (which is straightforward) or make other some other changes to achieve consistent behaviour.

RadhepS commented 1 week ago

@sam-garland Sorry for the delay! The PR has been approved. Next time you can send me a direct message on slack or an email! I often get too many emails for github so they get ignored.