-
Notifications
You must be signed in to change notification settings - Fork 4
fix: svelte gesture dep usage #365
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
base: main
Are you sure you want to change the base?
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughRemoved placeholder dependencies from platforms/pictique/package.json. Migrated Drawer.svelte swipe handling from the old swipe directive to the new useSwipe API, wiring a downward swipe to trigger drawer destruction. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant D as Drawer.svelte
participant G as svelte-gestures useSwipe
U->>D: Swipe down gesture
D->>G: useSwipe listens to swipe
G-->>D: swipedown event
D->>D: downSwipeHandler()
D->>D: destroy/close drawer
note over D: Drawer closes on downward swipe (via useSwipe)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
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.
Actionable comments posted: 2
🧹 Nitpick comments (1)
platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (1)
21-23
: Consider consolidating duplicate drawer destruction logic.
downSwipeHandler()
duplicates the logic indismiss()
(lines 17-19). Both destroy the drawer with animation.You could simplify by reusing
dismiss()
:- function downSwipeHandler() { - drawer?.destroy({ animate: true }); - } + const downSwipeHandler = dismiss;Or if you prefer to keep them separate for clarity, the current implementation is also acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (2)
platforms/pictique/package.json
(0 hunks)platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte
(3 hunks)
💤 Files with no reviewable changes (1)
- platforms/pictique/package.json
import { cn } from '$lib/utils'; | ||
import { swipe } from 'svelte-gestures'; | ||
import type { SwipeCustomEvent } from 'svelte-gestures'; | ||
import { useSwipe, type GestureCustomEvent, type SwipeCustomEvent } from 'svelte-gestures'; |
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.
Remove unused type imports.
GestureCustomEvent
and SwipeCustomEvent
are imported but not referenced anywhere in the code.
Apply this diff to remove the unused imports:
- import { useSwipe, type GestureCustomEvent, type SwipeCustomEvent } from 'svelte-gestures';
+ import { useSwipe } from 'svelte-gestures';
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
import { useSwipe, type GestureCustomEvent, type SwipeCustomEvent } from 'svelte-gestures'; | |
import { useSwipe } from 'svelte-gestures'; |
🤖 Prompt for AI Agents
In platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte around line 6, the
import currently brings in unused type symbols GestureCustomEvent and
SwipeCustomEvent; remove those unused type imports so the line only imports
useSwipe (and any other actually used exports) to eliminate dead imports and
TypeScript lint warnings.
{...useSwipe( | ||
() => {}, | ||
() => ({ | ||
timeframe: 300, | ||
minSwipeDistance: 60 | ||
}), | ||
{ | ||
onswipedown: downSwipeHandler | ||
} | ||
)} |
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.
🧩 Analysis chain
Verify the useSwipe API usage pattern.
The useSwipe
call has an empty first parameter () => {}
and wraps the configuration in a function. Confirm this matches the svelte-gestures API documentation for correct usage.
Please search the web to verify the correct useSwipe API signature:
🌐 Web query:
svelte-gestures useSwipe API documentation and usage examples
💡 Result:
Summary — useSwipe (svelte-gestures)
-
Import:
import { useSwipe, type SwipeCustomEvent, type GestureCustomEvent } from 'svelte-gestures'. [1][2] -
Hook signature / usage pattern:
Spread the action result onto an element: {...useSwipe(handler, () => (options))}. The first arg is your event handler (receives a SwipeCustomEvent); second is a function that returns options. Example below. [1] -
Events & detail payload:
- Main event: "swipe" — event.detail contains at least: direction (string: 'left'|'right'|'up'|'down'), pointerType, target (HTMLElement). There are also per-phase events like swipeup / swiperight / swipedown / swipeleft and gesture-phase events with names like swipeup, swipedown, swipemove that carry GestureCustomEvent payload (includes native pointer event, pointersCount, x, y, target). [2][1]
-
Common options (examples shown in README):
- timeframe: number (ms) — window in which movement counts as swipe.
- minSwipeDistance (or minSwipe): number (px) — minimum distance required to trigger swipe.
- touchAction: CSS touch-action to set on the element (e.g. 'none').
(Options are passed via the options function you give to useSwipe.) [1][2]
Minimal example
<script lang="ts"> import { useSwipe, type SwipeCustomEvent } from 'svelte-gestures'; let dir: string | null = null; function handler(e: SwipeCustomEvent) { dir = e.detail.direction; // e.detail.pointerType, e.detail.target available } </script>References
- svelte-gestures usage & useSwipe example (README / package docs). [1]
- svelte-gestures npm page (API events, options notes). [2]
[1] svelte-gestures README / docs (usage example).
[2] svelte-gestures — npm package page (API events & options).
Remove unsupported third argument from useSwipe
In platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte (lines 48–57), call useSwipe(handler, () => options)
only; move onswipedown={downSwipeHandler}
into the element’s markup (e.g. <div on:swipedown={downSwipeHandler} {...useSwipe(...)}>
).
🤖 Prompt for AI Agents
In platforms/pictique/src/lib/fragments/Drawer/Drawer.svelte around lines 48–57
the call to useSwipe incorrectly passes a third argument for onswipedown
(unsupported); change the invocation to useSwipe(() => {}, () => ({ timeframe:
300, minSwipeDistance: 60 })) and remove the third argument, then attach the
downSwipeHandler directly on the element that spreads useSwipe (e.g. add
on:swipedown={downSwipeHandler} to the wrapper div or the element receiving the
swipe directives) so the handler is invoked via Svelte event binding.
Description of change
Issue Number
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit