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.

Sign up for GitHub

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

Jump to bottom

[Storybook] Add stories for more components (letter P) - Part 2 #7674

Merged
merged 11 commits into from
Apr 26, 2024
Merged

[Storybook] Add stories for more components (letter P) - Part 2 #7674

merged 11 commits into from
Apr 26, 2024

Conversation

mgadewoll
Copy link
Contributor

@mgadewoll mgadewoll commented Apr 11, 2024 •
edited
Loading

Summary

closes #7481

This PR adds more stories for components with the letter P:

  • EuiInputPopover
  • EuiPopover
  • EuiPopoverTitle
  • EuiPopoverFooter
  • EuiWrappingPopover
  • EuiPortal
  • EuiProgress

QA

@mgadewoll mgadewoll added documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog labels Apr 11, 2024
@mgadewoll mgadewoll marked this pull request as ready for review April 11, 2024 17:01
@mgadewoll mgadewoll requested a review from a team as a code owner April 11, 2024 17:01
@mgadewoll mgadewoll force-pushed the storybook/7481-stories-p-part-2 branch from dc4a923 to ee3c402 Compare April 12, 2024 08:18
@cee-chen
Merge branch 'main' into storybook/7481-stories-p-part-2
92d05d0
@cee-chen
[EuiInputPopover] misc cleanup
59f936c 
- Fix `children` control
- remove unnecessary `closeOnScroll` jsx prop (should inherit from `...rest`
- disable `input` and `inputRef` props (not really possible to configure in controls)
@cee-chen
[EuiInputPopover] Add example that more clearly demonstrates `anchorP…
e93475f 
…osition`

+ combo'd with `panelMinWidth`

- requires moving the action toggles to just the playground, as an error will throw if `params.controls.include` is used with props that are not included
@cee-chen
[EuiPopover] Improve buffer arg
5023465
@cee-chen
[EuiPopover] Include panelPaddingSize context demo
c01f0d6 
- requires moving the action toggles to just the playground, as an error will throw if `params.controls.include` is used with props that are not included
@cee-chen
[EuiProgress] Match default false
55aaeb3
cee-chen
cee-chen approved these changes Apr 25, 2024
View reviewed changes
Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

I pushed up some optional minor prop preferences and extra stories for my spacetime week in lieu of actually review, hope that's okay! Feel free to merge whenever!

src/components/popover/popover_footer.stories.tsx Outdated
Copy link
Member

Choose a reason for hiding this comment

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

Awesome meta setup+use of decorators here! 🎉

src/components/popover/popover.tsx Show resolved Hide resolved
src/components/popover/popover.stories.tsx Outdated Show resolved Hide resolved
src/components/popover/popover.stories.tsx
Copy link
Member

Choose a reason for hiding this comment

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

[not a change request, just a note for future me] I'd like to eventually add stories for container, insert, repositionOnScroll as well! I'm too lazy for now so I just added a quick story to demo how panelPaddingSize affects child titles & footers

src/components/popover/wrapping_popover.stories.tsx

// NOTE: extended EuiPopoverProps are not resolved for some reason
// so we are currently manually adding them back
// TODO: remove this once types are properly resolved and added as control args
Copy link
Member

Choose a reason for hiding this comment

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

👍 Am personally good with these TODOs, EuiWrappingPopover is also generally kind of a weird component 😅 (also, I still need to look into button not being dynamically updatable...)

src/components/portal/portal.stories.tsx Outdated Show resolved Hide resolved
@mgadewoll
Copy link
Contributor Author

mgadewoll commented Apr 26, 2024 •
edited
Loading

I pushed up some optional minor prop preferences and extra stories for my spacetime week in lieu of actually review, hope that's okay! Feel free to merge whenever!

@cee-chen Thanks for the additional changes! They look good to me! 🎉

cee-chen
cee-chen approved these changes Apr 26, 2024
View reviewed changes
Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

🚢

@mgadewoll mgadewoll enabled auto-merge (squash) April 26, 2024 16:55
@kibanamachine
Copy link

Preview staging links for this PR:

@mgadewoll mgadewoll merged commit c4668a3 into elastic:main Apr 26, 2024
7 checks passed
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

@mgadewoll mgadewoll deleted the storybook/7481-stories-p-part-2 branch April 29, 2024 08:52
mgadewoll added a commit to mgadewoll/eui that referenced this pull request May 3, 2024
@mgadewoll @cee-chen
[Storybook] Add stories for more components (letter P) - Part 2 (elas…
061f3fa 
…tic#7674)

Co-authored-by: Cee Chen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cee-chen cee-chen cee-chen approved these changes

Assignees
No one assigned
Labels
documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[New docs] Create stories for components starting with the letter P
4 participants
@mgadewoll @kibanamachine @elasticmachine @cee-chen