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

docs: add new stories for UI components #8836

Merged
merged 5 commits into from
Sep 24, 2024

Conversation

benoitf
Copy link
Collaborator

@benoitf benoitf commented Sep 11, 2024

What does this PR do?

Add new stories for UI components

Screenshot / video of UI

image

https://66f17e753e2f921f3f94916e--podman-desktop-pr.netlify.app/storybook

What issues does this PR fix or reference?

fixes #7441

How to test this PR?

  • Tests are covering the bug fix or the new feature

@benoitf benoitf requested a review from a team as a code owner September 11, 2024 12:20
@benoitf benoitf requested review from dgolovin, cdrage and SoniaSandler and removed request for a team September 11, 2024 12:20
@axel7083
Copy link
Contributor

axel7083 commented Sep 11, 2024

The tooltip indication is not really visible in dark mode

image

I would remove the next Hover the icon and put a ? icon instead

Otherwise great job, I created #8840 and #8841 as I just noticed the problem with the numerous items added

Copy link
Contributor

@feloy feloy left a comment

Choose a reason for hiding this comment

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

  • I'm testing by running pnpm dev from the storybook directory, I'm not sure if there is a better way

  • In the Docs page, the Stories are not displaying anything (they are displayed correctly in dedicated pages):

stories-empty

  • In the source code, <wrapper is displayed instead of the real component name.

tooltip-source

  • could the terminal displaying the code be in light mode?

@benoitf
Copy link
Collaborator Author

benoitf commented Sep 12, 2024

@feloy I think the best way is to render the final look

or go to the website (click on the netlify link of this PR)

https://66e1a382d9435a070cc59d87--podman-desktop-pr.netlify.app/storybook/

@feloy
Copy link
Contributor

feloy commented Sep 12, 2024

@feloy I think the best way is to render the final look

or go to the website (click on the netlify link of this PR)

https://66e1a382d9435a070cc59d87--podman-desktop-pr.netlify.app/storybook/

ok, thanks. So the only remaining concern would be:

could the terminal displaying the code be in light mode?

@benoitf
Copy link
Collaborator Author

benoitf commented Sep 12, 2024

there might be a way but it's not related to this PR that only bring new stories (no terminal being added)

you can create the issue, it'll be for the storybook/docusaurus integration

@benoitf
Copy link
Collaborator Author

benoitf commented Sep 20, 2024

I've added a bunch of argsTypes parameters + example on selected for the tab

also added an extra padding on tooltip to see the tooltip

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.

LGTM

Copy link
Contributor

@dgolovin dgolovin left a comment

Choose a reason for hiding this comment

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

image

Is it normal for pr preview?

Also noticed that not all the elements have 'Basic' item. Not sure if that is intentional.

Copy link
Contributor

@feloy feloy 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 for the changes

fixes podman-desktop#7441
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
Signed-off-by: Florent Benoit <fbenoit@redhat.com>
@benoitf
Copy link
Collaborator Author

benoitf commented Sep 24, 2024

@dgolovin I fixed the dropdown rendering by adding a min-height and a padding

image

I renamed also Example to Basic

Copy link
Contributor

@dgolovin dgolovin left a comment

Choose a reason for hiding this comment

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

LGTM now

@benoitf benoitf merged commit 9e91864 into podman-desktop:main Sep 24, 2024
16 of 17 checks passed
@podman-desktop-bot podman-desktop-bot added this to the 1.13.0 milestone Sep 24, 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.

enhance storybook with new stories
5 participants