SSC: Fix notification colors#62860
Conversation
8c7ccea to
2e71693
Compare
chrsmith
left a comment
There was a problem hiding this comment.
I'll defer to Rob for approval. But this all looks good to me.
QQ: For common things like purple-success-alert, is CodyManageTeamPage.module.scss the right place to define these things?
Would we be better off if we just imported these styles from another CSS file?
e.g. maybe we keep CodyManageTeamPage.module.scss but just have an @import '../alerts'; statement so we don't need to duplicate these if we need to pop up alerts on another page?
|
My thinking is this:
So rather than future-proofing now, I want to organically transform the structure. It's very cheap, and the benefit is that each state is a valid one, plus, if we change our plans, we're still in a good place. |
|
I've updated the text colors: But then, with an eyedropper Chrome plugin, I couldn't confirm the real colors that get rendered because it seems to be broken even for the figma 🤯: But then, in the CSS, I use the correct dark theme background colors: I've also replaced some literals with the matching CSS variables. |
|
Thanks. I did not specify a red version anywhere, I think we should remove that for now. |
|
@rrhyne: Sure. What style shall I use for the "Invites not sent" error notification and its sisters? |
|
...or, broader question because I might be off-track with the whole notification direction: how should I communicate it when an action (like sending the invites) fails? |
|
Roger. I put together these warning states: We shouldn't go all red there as there isn't anything critical happening. |
|
@rrhyne the updated designs, now with the new icons: If it looks roughly right, could you stamp this PR? I can handle further tweaks in a follow-up, but I want to use these notifications on other pages, and for that, it'd be nice to have these new designs, even if they're not final. |
0ba8a09 to
495ad55
Compare
Our notifications had no dark theme style, and they weren't design-approved.
This PR makes them match the figma.
Caveats:
Light theme looks:
Dark theme looks:
Test plan
Tested with a temporary state to demo. See the screenshots.