feat: add viewer single-tap callbacks and tap events#158
Conversation
🦋 Changeset detectedLatest commit: ba80a15 The changes in this PR will be included in the next version bump. Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 49 minutes and 42 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThis PR introduces single-tap event handling to Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Deploying react-native-gesture-image-viewer with
|
| Latest commit: |
fb2c333
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://30f1a317.react-native-gesture-image-viewer.pages.dev |
| Branch Preview URL: | https://feat-single-tap-events.react-native-gesture-image-viewer.pages.dev |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb2c333291
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/useGestureViewerPaging.web.ts (1)
366-425: Consider extracting the 250ms double-tap arbitration window into a named constant.The
setTimeout(..., 250)at Line 410 is the single-tap arbitration delay used whenenableDoubleTapZoomis enabled. A named constant (e.g.WEB_SINGLE_TAP_DOUBLE_CLICK_WINDOW_MS = 250) at module scope would make the intent self-documenting and make it easy to tune later if browser double-click thresholds become an issue in practice.Also worth noting for reviewers: browsers fire
clickwithdetail: 1thendetail: 2on a double-click, and the 2nd event arrives after the 1st. The current cancel-then-schedule ordering correctly handles that cascade (detail===2 clears the pending detail===1 timer before applying zoom). No change needed there — just calling it out as an invariant worth a short inline comment if you want to document it.♻️ Suggested refactor
+const WEB_SINGLE_TAP_DOUBLE_CLICK_WINDOW_MS = 250; + type WebScrollActor = 'idle' | 'user' | 'autoplay';- webSingleTapTimerRef.current = setTimeout(() => { - onSingleTap(resolvedX, resolvedY); - webSingleTapTimerRef.current = null; - }, 250); + webSingleTapTimerRef.current = setTimeout(() => { + onSingleTap(resolvedX, resolvedY); + webSingleTapTimerRef.current = null; + }, WEB_SINGLE_TAP_DOUBLE_CLICK_WINDOW_MS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/useGestureViewerPaging.web.ts` around lines 366 - 425, Extract the hard-coded 250ms timeout in onWebClick into a module-scope named constant (e.g. WEB_SINGLE_TAP_DOUBLE_CLICK_WINDOW_MS = 250) and replace the setTimeout delay with that constant; update the dependency comments or add a short inline comment near onWebClick/webSingleTapTimerRef.current explaining the click detail (detail:1 then detail:2) arbitration so the intent is self-documenting and easy to tune later when using enableDoubleTapZoom and webSingleTapTimerRef.src/__tests__/manager.test.ts (1)
18-38: Consider covering a couple more tap-specific cases.The test validates the basic dispatch + unsubscribe path well. A couple of small additions would tighten the contract around
emitTap:
- Multiple
'tap'listeners: ensure all are invoked (and each unsubscribe is independent).- Payload independence: emit two different tap payloads and verify each listener receives the exact object it was emitted with (guards against accidental shared-object mutation if
emitEventever gets refactored).Not blocking — current test already validates the new
emitTapsurface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/__tests__/manager.test.ts` around lines 18 - 38, Add assertions to cover multiple 'tap' listeners and payload independence: instantiate GestureViewerManager, register two distinct tap listeners via addEventListener (e.g., tapListener and secondTapListener), emit two different tap payloads with emitTap and assert both listeners were called for each emission and received the exact payload objects passed; also unsubscribe one listener and verify the other still receives subsequent taps to confirm independent unsubscription (use unsubscribeTap and unsubscribeSecondTap variables to track).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/docs/2.x/en/guide/usage/gesture-viewer-props.mdx`:
- Around line 112-123: The example uses useState in the App component but the
import list only includes GestureViewer; update the example's imports to also
import useState from 'react' (add useState to the import directives before the
GestureViewer import) so the App function can call useState without error.
In `@docs/docs/2.x/en/guide/usage/handling-viewer-events.mdx`:
- Around line 61-65: The table for the viewer events incorrectly shows
previousScale and previousRotation as always numbers; update the documentation
for the zoomChange and rotationChange events to indicate that previousScale and
previousRotation can be null (e.g., `{ scale: number, previousScale: number |
null }` and `{ rotation: number, previousRotation: number | null }`) so the
signatures in the docs match the exported event types; locate the rows for
`zoomChange` and `rotationChange` in handling-viewer-events.mdx and change the
Callback Data cells to reflect the nullable types.
In `@docs/docs/2.x/ko/guide/usage/gesture-viewer-props.mdx`:
- Around line 112-123: The example uses the React hook useState but does not
import it; update the example to add an import for useState (e.g., import {
useState } from 'react') alongside the existing GestureViewer import so the
component function App can reference useState without error; ensure you update
both the Korean and English documentation files and keep the import line grouped
with the other imports.
In `@docs/docs/2.x/ko/guide/usage/handling-viewer-events.mdx`:
- Around line 61-65: Update the event table rows for zoomChange and
rotationChange to indicate that previousScale and previousRotation can be
nullable (number | null) instead of always number; locate the zoomChange and
rotationChange rows in the table and change the "콜백 데이터" column entries to `{
scale: number, previousScale: number | null }` and `{ rotation: number,
previousRotation: number | null }` respectively so the docs match the actual
types.
---
Nitpick comments:
In `@src/__tests__/manager.test.ts`:
- Around line 18-38: Add assertions to cover multiple 'tap' listeners and
payload independence: instantiate GestureViewerManager, register two distinct
tap listeners via addEventListener (e.g., tapListener and secondTapListener),
emit two different tap payloads with emitTap and assert both listeners were
called for each emission and received the exact payload objects passed; also
unsubscribe one listener and verify the other still receives subsequent taps to
confirm independent unsubscription (use unsubscribeTap and unsubscribeSecondTap
variables to track).
In `@src/useGestureViewerPaging.web.ts`:
- Around line 366-425: Extract the hard-coded 250ms timeout in onWebClick into a
module-scope named constant (e.g. WEB_SINGLE_TAP_DOUBLE_CLICK_WINDOW_MS = 250)
and replace the setTimeout delay with that constant; update the dependency
comments or add a short inline comment near
onWebClick/webSingleTapTimerRef.current explaining the click detail (detail:1
then detail:2) arbitration so the intent is self-documenting and easy to tune
later when using enableDoubleTapZoom and webSingleTapTimerRef.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c9e2c576-2f8b-4f94-bece-ae6ef8188507
📒 Files selected for processing (20)
.changeset/single-tap-events.mddocs/docs/2.x/en/guide/usage/basic-usage.mdxdocs/docs/2.x/en/guide/usage/gesture-viewer-props.mdxdocs/docs/2.x/en/guide/usage/handling-viewer-events.mdxdocs/docs/2.x/en/guide/usage/style-customization.mdxdocs/docs/2.x/ko/guide/usage/basic-usage.mdxdocs/docs/2.x/ko/guide/usage/gesture-viewer-props.mdxdocs/docs/2.x/ko/guide/usage/handling-viewer-events.mdxdocs/docs/2.x/ko/guide/usage/style-customization.mdxexample/src/Example.tsxsrc/GestureViewer.tsxsrc/GestureViewerManager.tssrc/__tests__/manager.test.tssrc/index.tsxsrc/types.tssrc/useGestureViewer.tssrc/useGestureViewerEvent.tssrc/useGestureViewerPaging.tssrc/useGestureViewerPaging.types.tssrc/useGestureViewerPaging.web.ts
1 participant
This release adds
onSingleTaptoGestureViewerso you can handle confirmed single taps without overlaying an extra pressable on top of the viewer.It also adds a
tapevent touseGestureViewerEvent, currently emitting confirmed single taps with{ kind: 'single', x, y, index }.This improves common viewer UI patterns such as toggling headers, toolbars, counters, or captions on tap while preserving swipe, pinch, dismiss, and double-tap zoom behavior.
Related discussion: #157
Summary by CodeRabbit
New Features
onSingleTapcallback prop to detect single-tap interactions on the viewer with tap coordinates (x, y) and item index.'tap'event support to event subscriptions with confirmed single-tap data.Documentation
Tests