Please include a description of this change and which issue it fixes. If no issue exists yet please include context and what problem it solves.
@justinvelluppillai will fix that one, you are 100% correct, test changed to
I don't see the changes yet so will remove the
needs review label until they're done @peterhashair 👍
@peterhashair I'm seeing
executePluginMethod('unload') once when initially switching tabs on a tracked page. Is this intentional?
My test was to add a
console.log(event); statement to the first line of
console.log('executePluginMethod'); after the
isPageUnloading check. Then load a tracked page and switch tabs. The first time the tab is switched
executePluginMethod is called, subsequent tab switches do not result in additional calls.
@bx80 yes, I found the same problem, mention on the issue itself as well,
visibilitychange event is a kind of design for mobile, triggers on changing tab, etc, but I guess that's expected.
@peterhashair I suppose existing plugin code would need to be aware that
executePluginMethod('unload') could now be called on a tab switch and not just on page unload. This might be a breaking change if code is relying on a single unload event being fired only when a page is closed.
For example, it looks like HSR will stop sending data the first time that the plugin unload event is called, so in a scenario where the user loaded the page, quickly switched to another tab to check mail and then switched back, then I suspect none of the subsequent user interactions would be recorded.
@bx80 makes sense, let me see if I can filter out the tab switch event.
@bx80 I am trying to do some research seems like that even can't escape tab change. Maybe that event shouldn't be added, any thought?
@peterhashair I took a look too and it doesn't look like the visibility API offers any way to differentiate between a tab switch and leaving the page, so we probably don't want to have this event trigger the plugin unload event and potentially break existing plugins.
@bx80 agree, but I notice, the visibiltyChange actually useful when is mobile.
beforeunload won't trigger for those.