| CARVIEW |
Navigation Menu
-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Add WithCloseHandlers story #69688
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
Add WithCloseHandlers story #69688
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
@ciampo please review this PR whenever you have time. Thanks! |
t-hamano
added
[Type] Developer Documentation
| }; | ||
|
|
||
| export const WithCloseHandlers: StoryObj< typeof Popover > = { | ||
| decorators: [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally speaking, I'd like to understand if we can abstract the decorator to avoid reperition with the default story — although it's definitely not a priority at the moment, let's first get the new example to look & work as expected.
|
Thanks for the extensive review @ciampo! I'll work on them 🙇♂️ |
|
Updated the PR 👍🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your patience iterating over this!
Code changes LGTM, we just need a CHANGELOG entry and then we'll be able to merge 🙏
|
Will update the changelog in the |
|
Updated the CHANGELOG.md |
|
Thank you, @Rishit30G ! As a follow-up, it would be great to try to refactor |
|
Thanks @ciampo for your proactive reviews and feedbacks! 💯
Well noted 📝 |
Likewise 👏🏻 👏🏻 |
* Add WithCloseHandlers story * Add focusOnMount * Update popover size and layout * Fix: replace decorator with render function * Modify event handlers and render functions * Refactor close and focus outside handlers in Popover story * Refactor event handler types and text in Popover story * Update focusOnMount argument * Add changelog and update focusOnMount props * Update changelog --- Co-authored-by: Rishit30G <rishit30g@git.wordpress.org> Co-authored-by: ciampo <mciampini@git.wordpress.org>
Labels
3 participants
What?
Closes #69675
Why?
This PR is necessary to add functionality for
onCloseandonFocusOutsideHow?
This PR adds a new story
WithCloseHandlerswhich will showcase the functionality ofonCloseandonFocusOutsideTesting Instructions
npm run storybook:devPopoverstoryWithCloseHandlersstory to check theonCloseandonFocusOutsidefunctionalityScreencast
Screen.Recording.2025-03-26.at.1.37.46.PM.mov