-
Notifications
You must be signed in to change notification settings - Fork 215
feat(product tours): click element to advance to next step #2783
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
feat(product tours): click element to advance to next step #2783
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This stack of pull requests is managed by Graphite. Learn more about stacking. |
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.
5 files reviewed, 1 comment
| const handleSpotlightClick = (e: MouseEvent) => { | ||
| e.stopPropagation() | ||
| if (targetElement) { | ||
| targetElement.click() | ||
| } | ||
| onNext() | ||
| } |
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.
logic: onNext() called synchronously after targetElement.click() may cause race condition. The click event hasn't fully propagated yet (event bubbling, async handlers, DOM updates). While retry logic exists, consider adding a small delay:
| const handleSpotlightClick = (e: MouseEvent) => { | |
| e.stopPropagation() | |
| if (targetElement) { | |
| targetElement.click() | |
| } | |
| onNext() | |
| } | |
| const handleSpotlightClick = (e: MouseEvent) => { | |
| e.stopPropagation() | |
| if (targetElement) { | |
| targetElement.click() | |
| } | |
| // Give click event time to propagate before advancing | |
| setTimeout(onNext, 0) | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: packages/browser/src/extensions/product-tours/components/ProductTourTooltip.tsx
Line: 243:249
Comment:
**logic:** `onNext()` called synchronously after `targetElement.click()` may cause race condition. The click event hasn't fully propagated yet (event bubbling, async handlers, DOM updates). While retry logic exists, consider adding a small delay:
```suggestion
const handleSpotlightClick = (e: MouseEvent) => {
e.stopPropagation()
if (targetElement) {
targetElement.click()
}
// Give click event time to propagate before advancing
setTimeout(onNext, 0)
}
```
How can I resolve this? If you propose a fix, please make it concise.|
Size Change: +785 B (+0.01%) Total Size: 5.26 MB
ℹ️ View Unchanged
|
ebd514c to
88ad02a
Compare
dfd885f to
c2ce7ca
Compare
88ad02a to
8a11136
Compare
c2ce7ca to
ee5e76d
Compare
8a11136 to
f145e2c
Compare

Problem
product tours only support progression through a "next" button, which means you cannot build a tour that requires any user interaction (e.g. opening a drawer or modal, etc)
Changes
adds support for a new
click-type trigger that will progress the tour when the target element is clicked. after click, the tour waits up to 2s for the next step's element to be visible.also:
exact(PR to add this note in main repo coming soon)product-tour-click-action.mp4 (uploaded via Graphite)
Release info Sub-libraries affected
Libraries affected
Checklist
If releasing new changes
pnpm changesetto generate a changeset file