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

2980-Fix: CommandGroup background #2985

Merged
merged 5 commits into from
Dec 19, 2023

Conversation

Kanav-Arora
Copy link
Contributor

Closes #2980

@charlesBochet
Copy link
Member

@Kanav-Arora Let's introduce a new boxShadow in theme: superHeavy.

Light mode:
box-shadow: 0px 0px 8px 0px rgba(0, 0, 0, 0.16), 0px 8px 64px -16px rgba(0, 0, 0, 0.48), 0px 24px 56px -16px rgba(0, 0, 0, 0.08);

Dark mode:
box-shadow: 2px 4px 16px 0px rgba(0, 0, 0, 0.12), 0px 2px 4px 0px rgba(0, 0, 0, 0.04);

(using grayScale.gray100 as others)

I've also noticed that boxShadow extraLight is not used anymore in the code and not part of Figma, let's remove it

@Kanav-Arora
Copy link
Contributor Author

Hi @charlesBochet
I have done the suggested changes but the shadow is too strong for light mode and looks same for dark mode.
The main reason for this issue was to improve dark mode. IG you can give it a thought once.
Still I have made the changes so you can check once.

@Kanav-Arora Kanav-Arora marked this pull request as ready for review December 15, 2023 16:50
@charlesBochet
Copy link
Member

@Kanav-Arora could you add screenshots?
@Bonapara can we get your input on it?

@Kanav-Arora
Copy link
Contributor Author

@charlesBochet PFA

Screenshot 2023-12-15 at 10 31 48 PM Screenshot 2023-12-15 at 10 32 14 PM

@charlesBochet charlesBochet added blocked: design needed This doesn't seem right and removed PR: awaiting author labels Dec 15, 2023
@Bonapara
Copy link
Member

@Kanav-Arora, thank you for your work on this. In addition to fixing the box shadow, we also need to correct the background colors. Here's what we can do:

  • Change the popup background color to secondary.
  • Remove the background color from the section titles.

image

https://www.figma.com/file/xt8O9mFeLl46C5InWwoMrN/Twenty?type=design&node-id=6639-72012&mode=design&t=77tjN1WZX11BpPQj-4

@charlesBochet charlesBochet added PR: awaiting author and removed blocked: design needed This doesn't seem right labels Dec 18, 2023
@Kanav-Arora
Copy link
Contributor Author

Hi @charlesBochet
Done

Copy link
Member

@charlesBochet charlesBochet left a comment

Choose a reason for hiding this comment

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

Thank you! I have made last changes based on discussion with @Bonapara

@charlesBochet charlesBochet merged commit ffcdace into twentyhq:main Dec 19, 2023
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CommandMenu design fix
3 participants