pinterest-for-woocommerce
pinterest-for-woocommerce copied to clipboard
:bulb: Idea for catching click events to fire Tracks
Is your feature request related to a problem?
This is a follow-up from the discussion at https://github.com/woocommerce/pinterest-for-woocommerce/pull/266#issuecomment-977669697, where we were looking for a better way to fire track events upon clicking on a link, without the risk of onClick
prop overwrite.
How to reproduce the problem
<MyElement
onClick={}
{ ...documentationLinkProps() }
onClick={}
/>
Describe the solution you'd like
@eason9487 wrote:
a new idea might be considered, but only for this kind of event tracking, not like the generalized pattern of Component hooks. […] The idea is to use a single global click event with
capture
type, then it doesn't need to bindonClick
prop on the target React element. So it'd not overwrite others or be overwritten by others. But the limitation is it has to be a react hook.
import { uniqueId } from 'lodash';
import { useRef, useEffect } from '@wordpress/element';
import { recordEvent } from '@woocommerce/tracks';
const DATA_ATTR = 'data-pwf-doc-link-id';
const payloadMap = new Map();
function handleClick( e ) {
const id = e.target.getAttribute( DATA_ATTR );
const payload = payloadMap.get( id );
if ( payload ) {
recordEvent( 'pfw_documentation_link_click', payload );
}
}
export default function useDocLinkTracking( {
href,
linkId,
context,
target = '_blank',
rel = 'noopener',
} ) {
const idRef = useRef( null );
if ( ! idRef.current ) {
idRef.current = uniqueId();
}
useEffect( () => {
if ( payloadMap.size === 0 ) {
document.defaultView.addEventListener( 'click', handleClick, true );
}
const id = idRef.current;
payloadMap.set( id, {
link_id: linkId,
context,
href,
} );
return () => {
payloadMap.delete( id );
if ( payloadMap.size === 0 ) {
document.defaultView.removeEventListener(
'click',
handleClick,
true
);
}
};
}, [ href, linkId, context ] );
return {
[ DATA_ATTR ]: idRef.current,
href,
target,
rel,
};
}
Considering that registering a global click event for each tracking usages should not be a problem. So the click event could be added and removed inside the
useDocLinkTracking
hook, therefore, theif ( payloadMap.size === 0 )
checks and thepayloadMap
map are unneeded.
Additional context
Interesting idea.
What I like here is:
- a global approach to catch all documentation links with one module/entity,
- native API
But there is also a number of things that bothers me:
- On one hand, it's global, but it still applies an event handler for every link - to me it seems redundant. If we already attach the event listener to the
document
, that checks the sharedpayloadMap
we could actuallyaddEventListener
it once. - I'm not sure how it would interact with other React-bound events. Does it always translate
onClick
props to the bubbling phase listeners? - I'd rather use
ownerDocument
rather thandocument
to make sure we target the correct one. - I'm not sure if assigning the event listeners using native APIs is fine in a functional React world
With native custom elements, I've been using a similar thing in the past, with a code like:
<any-element-in-shadow-or-document>
<track-clicks/>
…
<div>…<a href="foo" my-opt-in-attr="some-payload">Read more</a>
…
</any-element-in-shadow-or-document>
Then my-link-catcher
when connected was listening and captur
ing all click events in the scope of its parent node (any-element-in-shadow-or-document
). Then was checking if the target element has opt-in attribute (or didn't have opt-out attribute - depending on strategy), if it was ok, it was performing an action with the given payload.
It was more-or-less smth like:
class TrackClicks extends HTMLElement {
connectedCallback() {
this.parentNode.addEventListener('click', this, true);
}
disConnectedCallback() {
this.parentNode.removeEventListener('click', this);
}
handleEvent( event ){
if( event.target.hasAttiribute( 'data-pwf-doc-link-id' ) ){
recordEvent( 'pfw_documentation_link_click', {
linkId: event.target.getAttiribute( 'data-pwf-doc-link-id' ),
context: event.target.getAttiribute( 'data-pwf-doc-context' ),
href: event.target.href
});
}
}
};
For sure it does not fit the React world, but the idea is to
- add/remove the listener once
- have catching declaratively scoped
- work across shadow roots & external documents
- easy to use - just add an attribute to your link (no need for
onClick
, or hook)