fix(#3743): tertiary background colour fix on mobile#3768
Conversation
|
If this is as per design then it looks good. @twjeffery ? |
There was a problem hiding this comment.
Not sure if we should fix this on this PR, or maybe we can do in a separate one: When I focus on the button I see this, it is difficult to read the text and I think the text color should be white?
cc @twjeffery
mxsoco
force-pushed
the
mxsoco/3743-tertiary-button-mobile-background
branch
from
9f8b660 to
4441791
Compare
mxsoco
force-pushed
the
mxsoco/3743-tertiary-button-mobile-background
branch
from
4009dbd to
e3e9c76
Compare
bdfranck
left a comment
bdfranck
left a comment
There was a problem hiding this comment.
We can update and add tokens instead of removing them to prevent future issues.
| width: 100%; | ||
| display: flex; | ||
| } | ||
| button.tertiary { |
There was a problem hiding this comment.
We should update the token value instead of removing it. Removing it could lead to frustrated teams who update the -goa-button-tertiary-color-bg-mobile value and find that nothing happens.
| background-color: var(--goa-button-tertiary-color-bg-mobile) !important; | ||
| } | ||
| button.tertiary:hover { | ||
| background-color: var(--goa-color-greyscale-200) !important; |
There was a problem hiding this comment.
Let's add a new token with the value we want instead of removing the CSS here.
There was a problem hiding this comment.
If I update --goa-button-tertiary-color-bg-mobile to just be transparent, could that just replace --goa-color-greyscale-200 here?
✅ Deploy Preview for benji-docs-previews ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
bdfranck
left a comment
bdfranck
left a comment
There was a problem hiding this comment.
I looked at the latest changes with tokens from (https://github.com/GovAlta/design-tokens/pull/153/changes)...
mobile-tertiary-buttons-fixed.mp4
I see the tertiary button background is transparent on mobile. Looks good to me! 👍
|
🎉 This PR is included in version 2.0.0-dev.4 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 2.0.0-dev.11 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 7.0.0-dev.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version 5.0.0-dev.5 🎉 The release is available on: Your semantic-release bot 📦🚀 |
4 participants
Before (the change)
The background colour still exists on mobile for the tertiary button. Same with the inverse, but only on mobile.
After (the change)
Removed 4 lines that bring in the background. Including tokens:
--goa-button-tertiary-color-bg-mobile--goa-button-tertiary-color-bgSteps needed to test