Skip to content

refactor: Initial color palette changes #3087

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Jul 21, 2022
Merged

refactor: Initial color palette changes #3087

merged 4 commits into from
Jul 21, 2022

Conversation

BrunoQuaresma
Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma commented Jul 21, 2022

Screen Shot 2022-07-21 at 10 15 51

Screen Shot 2022-07-21 at 10 14 44

@BrunoQuaresma BrunoQuaresma requested a review from a team as a code owner July 21, 2022 13:07
@BrunoQuaresma BrunoQuaresma self-assigned this Jul 21, 2022
@BrunoQuaresma
Copy link
Collaborator Author

@hkfoster I see the colors are in hsl, any specific reasons for that?

@@ -48,5 +48,6 @@ const useStyles = makeStyles((theme) => ({
},
avatar: {
marginRight: theme.spacing(1.5),
background: "hsl(219, 8%, 52%)",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we could have it as a palette object like theme.palette.paper.dark? Or something like that?

@BrunoQuaresma
Copy link
Collaborator Author

@hkfoster I also see some colors being used directly into the components, ideally IMO, they should be a color in the palette object. It could be something simple like blue.500 blackAlpha.100 etc. What do you think?

@BrunoQuaresma
Copy link
Collaborator Author

The new colors look nice tho, great job!

@BrunoQuaresma
Copy link
Collaborator Author

Did you use the colors from here?
Screen Shot 2022-07-21 at 10 18 20

@Kira-Pilot
Copy link
Member

😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍 😍

@Kira-Pilot
Copy link
Member

I left a comment in the snapshots associated with this view, but looks like we lost the darkened box around our code snippet:

Screen Shot 2022-07-21 at 10 13 26 AM

@Kira-Pilot
Copy link
Member

Just a thought: should we be using a darkened error color? This looks a little bright to me, but maybe it's better that way.
Screen Shot 2022-07-21 at 10 02 18 AM

@BrunoQuaresma
Copy link
Collaborator Author

@Kira-Pilot about the Dialog, I think we could keep it as it is for now, and about the dark color in the code example, I will fix that.

@BrunoQuaresma
Copy link
Collaborator Author

cc. @Kira-Pilot
Screen Shot 2022-07-21 at 13 45 43

@kylecarbs
Copy link
Member

I'm very excited for this 😍

@BrunoQuaresma
Copy link
Collaborator Author

I forgot to update the copy button color:
Screen Shot 2022-07-21 at 14 31 05

@BrunoQuaresma BrunoQuaresma enabled auto-merge (squash) July 21, 2022 17:51
@BrunoQuaresma BrunoQuaresma merged commit 5b78251 into main Jul 21, 2022
@BrunoQuaresma BrunoQuaresma deleted the color-palette branch July 21, 2022 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants