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

Optimize whitespace in dependencies widget #262

Merged
merged 3 commits into from
Aug 10, 2023

Conversation

steff456
Copy link
Contributor

@steff456 steff456 commented Aug 3, 2023

Fixes #242

This PR,

  • Removes bottom margin rule from the dependencies widget

Screenshots

Before
image

After
image

Questions

  • Do we want to increase the height of the widget? That would mean we need to change the surrounding areas to use flex for it to be properly rendered. In case we want to go that way, I would suggest to do it in a follow up PR.

@steff456 steff456 added type: enhancement 💅🏼 New feature or request area: UI design 🎨 Items related to the user interface impact: high 🟥 This issue affects most of the nebari users or is a critical issue project: JATIC Work item needed for the JATIC project labels Aug 3, 2023
@steff456 steff456 added this to the 🚀 JATIC - Q1 milestone Aug 3, 2023
@steff456 steff456 self-assigned this Aug 3, 2023
@kcpevey
Copy link
Contributor

kcpevey commented Aug 8, 2023

I think because the rest of the UI has so much whitespace, I'd like to add just a little bit back in between the list entries. Maybe just 2 pxs?

Also, the issue also includes the sizing of the widget box. Id like it to be twice as tall as it is right now.

@steff456
Copy link
Contributor Author

steff456 commented Aug 9, 2023

This is the behavior of the last commit,

scroll

@pierrotsmnrd
Copy link
Contributor

That works for me but I noticed that we were having a different result between the two themes, green-accent and grayscale. The list isn't "dotted" anymore (see screenshots below)

You can switch the color theme by editing your .env file and changing the value for REACT_APP_STYLE_TYPE to any of the following :

REACT_APP_STYLE_TYPE=green-accent
REACT_APP_STYLE_TYPE=grayscale

I guess that's not the consequence of your PR but the consequence of another PR. Can you check if you find a PR related to this, and fix it so we have a coherent design on both themes please ?

result with green-accent:

green-accent

result with grayscale:

grayscale

@steff456
Copy link
Contributor Author

That works for me but I noticed that we were having a different result between the two themes, green-accent and grayscale. The list isn't "dotted" anymore (see screenshots below)

Yes @pierrotsmnrd! I also noticed that the themes don't match. I'll create a new issue for this. I also found that having a box is a little bit weird because I thought they were checkboxes at first. Maybe we need to have a discussion on how we want this to look in both themes.

@pierrotsmnrd
Copy link
Contributor

I agree they look like checkboxes, they should be removed.
Ok, open another issue. Once done, you can merge this one. I'm marking it ready for merge

Copy link
Contributor

@pierrotsmnrd pierrotsmnrd left a comment

Choose a reason for hiding this comment

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

Ready for merge. The slight difference between both themes as described above will be handled in a different issue

@steff456
Copy link
Contributor Author

New issue at: #266

Thanks for the review!

@steff456 steff456 merged commit 654b3a5 into conda-incubator:main Aug 10, 2023
1 check passed
@steff456 steff456 deleted the fix-242 branch August 10, 2023 14:23
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 impact: high 🟥 This issue affects most of the nebari users or is a critical issue project: JATIC Work item needed for the JATIC project status: merge ready 🚀 type: enhancement 💅🏼 New feature or request
Projects
Status: Done 💪🏾
Development

Successfully merging this pull request may close these issues.

Packages installed as dependencies widget is too small
3 participants