feat(dropdown): add show/hide events #301
Conversation
WalkthroughThe recent updates enhance the dropdown component with comprehensive API documentation, introducing new props, events, and slots. Additionally, the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
✅ Deploy Preview for sensational-seahorse-8635f8 ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- docs/components/dropdown.md (1 hunks)
- src/components/FwbDropdown/FwbDropdown.vue (2 hunks)
Additional comments not posted (6)
src/components/FwbDropdown/FwbDropdown.vue (3)
47-47: LGTM: Necessary imports added.The added imports
computed,ref,toRef, andwatchfrom Vue are necessary for the new functionality and are correctly included.
77-81: LGTM: Necessary emits added.The added emits for
showandhideevents are necessary to notify when the dropdown is shown or hidden and are correctly included.
82-91: LGTM: Watch block forvisiblestate added.The watch block for the
visiblestate correctly emitsshoworhideevents based on the visibility of the dropdown.docs/components/dropdown.md (3)
263-270: LGTM: Props documentation added.The added props
placement,text,transition,closeInside, andalignToEndare necessary for the new functionality and are correctly documented.
272-276: LGTM: Events documentation added.The added events
showandhideare necessary to notify when the dropdown is shown or hidden and are correctly documented.
278-282: LGTM: Slots documentation added.The added slots
defaultandsuffixprovide flexibility for customizing the dropdown content and are correctly documented.
cogor
added
🔧 enhancement
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- src/components/FwbDropdown/FwbDropdown.vue (2 hunks)
Additional comments not posted (3)
src/components/FwbDropdown/FwbDropdown.vue (3)
47-47: LGTM!The import statement for
watchis correct and necessary for the new functionality.
77-80: LGTM!The
emitdeclaration forshowandhideevents is correct and aligns with the new functionality.
Line range hint
47-92:
LGTM!The changes integrate well within the existing code structure, enhancing the functionality of the dropdown component.
| watch( | ||
| visible, | ||
| (isVisible: boolean) => { | ||
| if (isVisible) { | ||
| emit('show') | ||
| } else { | ||
| emit('hide') | ||
| } | ||
| }, | ||
| ) |
There was a problem hiding this comment.
LGTM! Ensure proper test coverage.
The watch function is well-implemented and enhances the component's interactivity by emitting show and hide events based on the visible state. Ensure that this new functionality is covered by tests.
Do you want me to generate the unit testing code or open a GitHub issue to track this task?
There was a problem hiding this comment.
i believe it's out of the scope of the PR to set up a bunch of new tests. but a task for testing in general is up to the maintainers, not me
There was a problem hiding this comment.
@Flambe, understood. I'll go ahead and open a GitHub issue to track the task of adding tests for the show and hide events. This will ensure that the new functionality is properly covered in the future.
Would you like me to proceed with creating the issue?
Learnings added
Learnt from: Flambe
PR: themesberg/flowbite-vue#301
File: src/components/FwbDropdown/FwbDropdown.vue:82-91
Timestamp: 2024-07-30T09:40:49.346Z
Learning: When a user indicates that setting up new tests is out of the scope of a PR, offer to open a GitHub issue to track the task of adding the necessary tests.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
Add show/hide events when opening/closing the dropdown as it can be useful to perform actions when the dropdown is toggled
Summary by CodeRabbit
placement,text,transition,closeInside, andalignToEnd.showandhideevents for improved interactivity of the dropdown component.defaultandsuffixslots to enhance dropdown customization.