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

Light mode for NavPage & FormPage #7149

Merged
merged 4 commits into from May 14, 2024

Conversation

feloy
Copy link
Contributor

@feloy feloy commented May 10, 2024

What does this PR do?

Adds support to light mode on NavPage & FormPage components

Screenshot / video of UI

NavPage

light-navpage

FormPage

light-formpage

What issues does this PR fix or reference?

Fixes #6889

@feloy feloy requested review from benoitf and a team as code owners May 10, 2024 07:21
@feloy feloy requested review from cdrage, lstocchi, deboer-tim and ekidneyrh and removed request for a team, cdrage and lstocchi May 10, 2024 07:21
Copy link
Contributor

@axel7083 axel7083 left a comment

Choose a reason for hiding this comment

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

Nice!

Copy link
Contributor

@ekidneyrh ekidneyrh left a comment

Choose a reason for hiding this comment

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

Nice! I'm still working on what colours to work for the missing components on here, but I'll update asap on the tracking ticket :)

@deboer-tim
Copy link
Collaborator

In the current design, forms have an intentional color change between the header area and the content. These color values remove that. @ekidneyrh is this an intentional design change? If so, do we need to keep a separate -bg color? The area between the title and start of form also looks big (since it's effectively double-padded), should we reduce this?

Screenshot 2024-05-10 at 8 05 35 AM

@ekidneyrh
Copy link
Contributor

Nav Page:
Nav Page.pdf
NavPage

Form Page:
Form Page
Content Theming.pdf

Yah I'm in favour of getting rid of of the extra padding and use the same background for the header and the rest of the form. I don't think there's a reason to have two different colours.

Is there any values im missing? I know there is some overlap with the image and volumes pages. I dont think there's a need to have a bg colour for the table-body-label-text on the nav pages.

@feloy
Copy link
Contributor Author

feloy commented May 14, 2024

Yah I'm in favour of getting rid of of the extra padding and use the same background for the header and the rest of the form. I don't think there's a reason to have two different colours.

I have added a commit to remove the top padding of the forms using the FormPage component

@feloy
Copy link
Contributor Author

feloy commented May 14, 2024

Nav Page: Nav Page.pdf NavPage

The table-body-label-text defined in #7151 was intended for the text Kubernetes/Podman in the Environment column (inside a label). You are reusing this name for the text of the header (which was defined by table-header-text in #7151). I'm ok to use table-body-label-text for the header text, but in this case it will be missing the color for the text in the Environment column.

deboer-tim added a commit to deboer-tim/desktop that referenced this pull request May 14, 2024
Adds light mode support to Link component, using color defined in design for
containers#7149.

Fixes containers#7215.

Signed-off-by: Tim deBoer <[email protected]>
@deboer-tim deboer-tim mentioned this pull request May 14, 2024
1 task
@feloy feloy merged commit c345039 into containers:main May 14, 2024
8 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.11.0 milestone May 14, 2024
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.

Light mode for NavPage & FormPage
5 participants