-
Notifications
You must be signed in to change notification settings - Fork 6
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
fix(rrweb): clean up pointer tap circles when seeking by breadcrumb #209
Conversation
c2ca93e
to
1e42bdb
Compare
// prevents multiple touch circles from staying on the screen | ||
// when the user seeks by breadcrumbs | ||
Object.values(this.pointers).forEach((p) => { | ||
if (p !== pointer && !p.touchActive) { |
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.
the !p.touchActive
ensures that this works for multitouch when seeking - all taps should show up
// when the user seeks by breadcrumbs | ||
Object.values(this.pointers).forEach((p) => { | ||
if (p !== pointer && !p.touchActive) { | ||
p.touchActive = false; |
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.
Hmm since the condition is !p.touchActive
, wouldn't this mean touchActive
is already false?
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.
if p.touchActive === true
(this can be true for various pointers at the same time in the case of multitouch) then we don't want to set touchActive = false
. otherwise we'd remove the pointer erroneously for multitouch. but touchActive
can also be null
for the pointers that aren't active, so those are the cases we want to set touchAcitve = false
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.
Can we clarify this in a code comment? (and also what it means for it to be null
vs false
)
isSync
is true) so that there aren't a bunch left on the screen at oncebefore:
Screen.Recording.2024-06-20.at.11.39.04.AM.mov
after (video also shows playback working normally):
Screen.Recording.2024-06-20.at.11.41.07.AM.mov