feat: add mousedown and mouseup events to Button component #1491
Conversation
|
@hi-rai is attempting to deploy a commit to the Themesberg Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request focus on the Changes
Assessment against linked issues
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
src/lib/buttons/Button.svelteOops! Something went wrong! :( ESLint: 9.15.0 TypeError: Error while loading rule '@typescript-eslint/no-unused-expressions': Cannot read properties of undefined (reading 'allowShortCircuit') 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 using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (2)
src/lib/buttons/Button.svelte (2)
150-152: LGTM: Mouse events properly added to button elementThe mouseup and mousedown events are correctly implemented, maintaining behavior parity with the anchor element implementation. Consider adding documentation about keyboard event handling for accessibility.
Consider adding a comment in the component documentation about keyboard event handling alternatives for these mouse events to ensure proper accessibility guidance.
Line range hint
165-175: Consider enhancing component documentationWhile the implementation is solid, the component's documentation section could benefit from:
- Documentation of the newly added mouse events
- Examples of common use cases (press-and-hold actions)
- Accessibility guidelines for keyboard alternatives
Would you like me to help generate enhanced documentation for these new events?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
src/lib/buttons/Button.svelte(3 hunks)
🔇 Additional comments (2)
src/lib/buttons/Button.svelte (2)
14-14: LGTM: Type definition now matches implementation
The addition of 'label' to the tag type union properly reflects the existing label support in the component's implementation, improving type safety.
142-144: LGTM: Mouse events properly added to anchor element
The mouseup and mousedown events are correctly implemented using Svelte's event forwarding syntax. Let's verify the event handling implementation across the codebase.
✅ Verification successful
Mouse events are properly implemented and consistent with existing usage
The mouseup and mousedown events added to the Button component align with existing event handling patterns in the codebase:
- Modal.svelte uses mousedown for outside click handling
- Carousel.svelte implements drag functionality using mousedown/mouseup events
- The events are consistently forwarded without modifiers where appropriate
The implementation follows established patterns and doesn't introduce any inconsistencies with existing mouse event handling in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent event handling patterns and usage
# Look for any custom event handlers that might need to handle these new events
# Search for mousedown/mouseup event handlers in component usage
rg -l "on:(mousedown|mouseup)" --type svelte
# Search for potential press-and-hold implementations that could benefit from these events
rg -l "setTimeout.*press" --type svelte
Length of output: 150
Script:
#!/bin/bash
# Search for mousedown/mouseup event handlers in component usage without file type restriction
rg -l "on:(mousedown|mouseup)"
# Search for potential press-and-hold implementations
rg -l "setTimeout.*press"
# Search for Button component usage to see if these events are used
rg -l "Button.*on:(mousedown|mouseup)"
# Look for any custom event handlers or event dispatching related to mouse events
rg "dispatch\(.*mouse(up|down)" -A 2
Length of output: 269
Script:
#!/bin/bash
# Check the context of mousedown/mouseup usage in the identified files
rg "on:(mousedown|mouseup)" src/lib/modal/Modal.svelte -C 2
rg "on:(mousedown|mouseup)" src/lib/carousel/Carousel.svelte -C 2
rg "on:(mousedown|mouseup)" src/routes/docs/components/dropdown.md -C 2
# Look for any Button component imports and usage patterns
rg "import.*Button" src/lib/modal/Modal.svelte src/lib/carousel/Carousel.svelte
Length of output: 1710
Closes #1490
📑 Description
Add mousedown and mouseup events to Button component
Status
✅ Checks
ℹ Additional Information
Summary by CodeRabbit
New Features
Bug Fixes
on:mouseupandon:mousedownevent listeners.