Skip to content
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

Add new colors and define green theme #292

Merged

Conversation

steff456
Copy link
Contributor

Part of #288

Description

This pull request:

  • Defines a green theme
  • Adds new objects with the colors defined in the color system
  • Picks the right theme depending on the preference setting

Pull request checklist

  • Did you test this change locally?
  • Did you update the documentation (if required)?
  • Did you add/update relevant tests for this change (if required)?

Additional information

  • MUI only defines three shades per color, light, main, dark so I picked three shades in the palette, this can be iterated later on another PR if necessary
  • MUI defines other roles for colors like error, info, success, etc. Do we want different colors for those roles or should I use the same accent color for all of them? This as well is something we can add in a follow up PR if necessary.

@steff456 steff456 added type: enhancement 💅🏼 New feature or request area: UI design 🎨 Items related to the user interface labels Sep 15, 2023
@steff456 steff456 added this to the 🚀 JATIC - Q1 milestone Sep 15, 2023
@steff456 steff456 requested a review from smeragoel September 15, 2023 14:21
@steff456 steff456 self-assigned this Sep 15, 2023
@trallard trallard self-requested a review September 15, 2023 16:58
@kcpevey kcpevey requested review from pavithraes and removed request for trallard September 19, 2023 16:10
src/theme.tsx Outdated Show resolved Hide resolved
src/theme.tsx Outdated Show resolved Hide resolved
Copy link
Member

@pavithraes pavithraes left a comment

Choose a reason for hiding this comment

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

LGTM - thanks, @steff456!

To confirm, we're only defining the theme here, correct? And, we're yet to update the UI to use this theme? (The UI seems to have hardcoded colours right now). Please feel free to merge if this is accurate. :)

@trallard
Copy link
Collaborator

Note I have an unfinished review

@steff456
Copy link
Contributor Author

To confirm, we're only defining the theme here, correct? And, we're yet to update the UI to use this theme? (The UI seems to have hardcoded colours right now). Please feel free to merge if this is accurate. :)

Yes! This is the base PR, I will update the use of the theme across the code base as I start implementing the changes for the design system per component

Copy link
Collaborator

@trallard trallard left a comment

Choose a reason for hiding this comment

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

Overall this is moving in the right direction, I just need Smera to confirm shades, but from our last conversations I think my comments are accurate

src/App.tsx Outdated Show resolved Hide resolved
Comment on lines +12 to +17
main: "#C4C4C4",
contrastText: white
},
secondary: {
main: "#7E7E7E"
main: "#7E7E7E",
contrastText: white
Copy link
Collaborator

Choose a reason for hiding this comment

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

@smeragoel will have to confirm here the exact colours

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a blocker for this PR or can we add the colors for the grayscale theme in a follow up PR?

Copy link
Collaborator

Choose a reason for hiding this comment

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

nope we can merge this and implement this later

src/theme.tsx Outdated Show resolved Hide resolved
src/theme.tsx Outdated Show resolved Hide resolved
src/theme.tsx Outdated Show resolved Hide resolved
@steff456 steff456 mentioned this pull request Sep 21, 2023
4 tasks
Comment on lines +12 to +17
main: "#C4C4C4",
contrastText: white
},
secondary: {
main: "#7E7E7E"
main: "#7E7E7E",
contrastText: white
Copy link
Collaborator

Choose a reason for hiding this comment

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

nope we can merge this and implement this later

@trallard trallard merged commit 9ca3ef4 into conda-incubator:design-system-implementation Sep 25, 2023
1 check passed
trallard pushed a commit that referenced this pull request Nov 27, 2023
* Add new colors and define green theme

* Fix primary button style

* Add new colors for roles and fix the shade used previously

* update main shade of color

* Fix icon buttons behavior and use across the UI

* Change name to condaStoreTheme, define more colors and refactor baseTheme

* Add rule for component in the baseTheme

* ENH -  Redefine new themes and colours (#292)

* Add new colors and define green theme

* Add new colors for roles and fix the shade used previously

* Update main shade of color

* Change name to condaStoreTheme, define more colors and refactor baseTheme

* Set input focus in purple

* Adjust all input components to design system using green

* Add new style for disabled buttons and hover state

* Implement new toggle switch design

* Fix border colors across the UI

* add missing semicolon to fix linting issues

* Add new design to links (#320)

* ENH - Add new sidebar design (#331)

* Add new style to left sidebar

* define react_app_version env variable

* Add custom icons, fix typography in link and do style changes from review

* Add trailing whitespace to fix lint error

---------

Co-authored-by: Christopher Ostrouchov <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: UI design 🎨 Items related to the user interface status: merge ready 🚀 type: enhancement 💅🏼 New feature or request
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

4 participants