Introducing consistency in colors for deposits across pages #8382
Conversation
naman03malhotra
changed the title
naman03malhotra
changed the title
|
@rogermattic can you please review the colors as part of this change? |
Test the buildOption 1. Jetpack Beta
Option 2. Jurassic Ninja - available for logged-in A12s🚀 Launch a JN site with this branch 🚀 ℹ️ Install this Tampermonkey script to get more options. Build info:
Note: the build is updated when a new commit is pushed to this PR. |
|
Team, I want to double-check, as:
|
|
Size Change: -10 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
Manual testThe colors seem as expected for This is not for us, but a general feedback on the WP color. This may be my eyesight or the screen too. The dark green appears not very contrastingly different from the black font. Strangely on the screenshot it looks better, but not in the actual browser.
|
| } | ||
|
|
||
| &.is-pending, |
There was a problem hiding this comment.
I couldn't find a canceled state showing in grey in the issue description.
There is an image shared in the issue with slightly different brighter shades ( probably studio ) . I had to recopy this image here since it had some weird private URL.
We may want to use the wp-admin shade equivalents, that are slightly darker / a bit dull compared to the screenshot in the issue. @rogermattic may kindly reconfirm.
Secondly, in transit and pending states are both yellow.
As per the Acceptable criteria in the issue, we need to use shades of blue for in transit.
So maybe
| &.is-pending, | |
| &.is-pending { | |
| background: $wp-blue-70; | |
| border-color: $wp-blue-0; | |
| } | |
| &.is-canceled { | |
| background: $wp-gray-70; | |
| border-color: $wp-gray-blue; | |
| } |
The gray color may end up looking almost black, though.
We probably need a different shade?
|
Left a couple of comments. Re-inviting Helix team too since I was unable to do a manual test for |
There was a problem hiding this comment.
What I understand about the ask from the issue #7761 is that we need to have a consistent coloring for each of the deposit state shown on the deposits list page and deposits details page.
So here's what I did to confirm that:
- Check the status color on the list page
- Make a local change here and pass status
paid,pending,in_transit,failed, orcanceled, so I don't have to create deposit in all those possible states. - I inspect the status and see the color code, eg for
paidstatus, the chip element has color#005c12.
- Make a local change here and pass status
- Check the status color on the details page
- Make a local change here and pass status
paid,pending,in_transit,failed, orcanceled, so I don't have to create deposit in all those possible states. - I inspect the status and see the background color code, eg for
paidstatus, theis-paidclass has background color#005c12.
- Make a local change here and pass status
<OrderStatus order={ { status: "paid" } } orderStatusMap={ displayStatus } />
Here are what I found that need fixing:
- When status is
in_transit, the details page shows yellow where it should be green like shown on the deposits list page. Also, I think settingwhite-space: nowrapwould be a good idea.
Here's how it's shown on the list page:
Here's how it's shown on the details page:
- When status is
canceled, the details page shows grey where it should be red like shown on the deposits list page.
Here's how it's shown on the list page:
Here's how it's shown on the details page:
Also, I think you are aware already but I like to mention that there are WordPress colours defined as constants here in the repo. I just learned about this as pointed out by @Jinksi's here p1709158123401059/1709146807.215419-slack-C02BW3Z8SHK.
|
Just came across this – does it need work or is it ready for merge? @shendy-a8c can you take a look and make a plan to follow up (yourself or assign)? |
|
Thank you for checking, @haszari. It needs work. It's on my radar but pretty low compared to other things on my plate. ETA is late this week or next week. |
Fixes #7761
Changes proposed in this Pull Request
Here is the result after the change:
Testing instructions
Align deposit status colour coding across pages #7761 (comment)
npm run changelogto add a changelog file, choosepatchto leave it empty if the change is not significant. You can add multiple changelog files in one PR by running this command a few times.Post merge