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

Fully built out v6 demo #209

Open
wants to merge 44 commits into
base: v6-penta-demo
Choose a base branch
from

Conversation

evwilkin
Copy link
Collaborator

@evwilkin evwilkin commented Feb 14, 2024

Towards #202

This PR adds the following items to the seed app for the V6 demo:

  • Dashboard page
  • Resources page
  • Inventory page
  • Dark/light theme toggle
  • RTL switch
  • Updated favicon

https://pf6-alpha2-demo.surge.sh/

@evwilkin evwilkin changed the base branch from main to v6-penta-demo February 14, 2024 00:05
@andrew-ronaldson
Copy link

Noticed these issues below:
The Events card on the Dashboard screen gets cut off.
image

On the same screen this utility classes don't change with dark theme
image

Lastly, there is an inline style on the spans. I think this should be using var(--pf-t--global--text--color--subtle)
Screenshot 2024-02-22 at 4 16 31 PM

@evwilkin evwilkin linked an issue Mar 19, 2024 that may be closed by this pull request
@evwilkin
Copy link
Collaborator Author

@andrew-ronaldson thanks for the feedback, connected with @mcoker (added as reviewer) for suggestions on fixes and fixed the overflowing grid item && update the incorrect text colors (note - now using bold tag rather than another color per suggestion).

Copy link

@mcoker mcoker left a comment

Choose a reason for hiding this comment

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

I think it looks fine, but I left some comments. I think the summary is something like:

  • Any existing utility classes will be based off of the old global vars. Spot checking, I think the "Text" utilities will be the most problematic. Flex, sizing, and accessibility utilities are probably fine the way they are and likely won't be any different in v6.
  • Any --pf-v5-global[...] vars will be from the old system and should ideally be replaced with --pf-t--global[...]. Looks like most of the v5 global vars reference in these files are for spacers, which is great, because the spacers are the same between v5 and v6, they just have a different --pf-t--global prefix.
  • Looks like we're importing colors from react-tokens, and those are referencing our old v5 colors. But looks like they're used to set icon colors, which is covered in the next bullet.
  • Any time we need to set a status color on an icon, we can use <Icon status="...">
  • Instead of .pf-v5-u-font-color-200 or text.color_200 (text is a reference to the text utility)....
    • The <Text> component will give you small/grey text using the small variant.
    • The --pf-t--global--text--color--subtle token will turn text grey/secondary in color (which you're already using some places, but not all) if you want the text to be grey but not small.

src/app/Dashboard/CardEventsView.tsx Outdated Show resolved Hide resolved
src/app/Dashboard/CardEventsView.tsx Outdated Show resolved Hide resolved
src/app/Dashboard/CardHorizontalGrid.tsx Outdated Show resolved Hide resolved
src/app/Dashboard/CardNested.tsx Outdated Show resolved Hide resolved
src/app/Dashboard/Dashboard.tsx Outdated Show resolved Hide resolved
src/app/Dashboard/RecommendationsCard.tsx Outdated Show resolved Hide resolved
src/app/Dashboard/CardStatus.tsx Outdated Show resolved Hide resolved
@evwilkin
Copy link
Collaborator Author

Thanks @mcoker, just FYI I went back and made those updates based on your suggestions 👍
@nicolethoen @andrew-ronaldson this demo PR should be in a good place now after feedback & fixes. The one outstanding obvious bug is the chart colors in dark theme, which I believe is yet to be updated for V6 so is as expected.

@nicolethoen
Copy link
Collaborator

This just needs to be updated to the latest alphas as soon as the official alpha versions have been announced

@dlabaj dlabaj added the demo label Apr 19, 2024
@andrew-ronaldson
Copy link

Screenshot 2024-05-21 at 12 46 35 PM

Thanks for the updates.
Seeing this blue bordered section on the page now.

@andrew-ronaldson
Copy link

That odd border still appears. Looks like it is from a :visible-focus on main 🤷‍♂️ .
Since the labels in the demo are indicating a status we should use p-m-danger/warning/success etc. instead of pf-m-red/green/orange etc.
Screenshot 2024-06-27 at 9 12 58 AM

@nicolethoen
Copy link
Collaborator

Latest surge preview with alpha2 versions: https://pf6-alpha2-demo.surge.sh/

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.

Dashboard page demo (Overview page)
8 participants