Doist / reactist

Open source React components made with ❤️ by Doist
http://doist.github.io/reactist
MIT License
250 stars 22 forks source link

fix: Set pointer-events none for button elements to ensure GA tracking #743

Closed engfragui closed 1 year ago

engfragui commented 1 year ago

Short description

While reviewing https://github.com/Doist/todoist-web/pull/5709, Ernesto suggested that, instead of sprinkling our todoist-web code with:

button[data-gtm-id] span {
    pointer-events: none;
}

and

a[data-gtm-id] span {
    pointer-events: none;
}

to make sure that our GA tracking works well on buttons, we can actually set that in Reactist ("at the source") so that we don't need to bother with it in todoist-web.

The result is that the inner span of any button element (both Reactist's Buttons and ButtonLinks) has the pointer-events: none property set:

Screenshot 2023-01-13 at 15 22 35

Some more background

If we don't make this change (a.k.a. if we don't add pointer-events: none to our buttons), the user click will happen on the span nested inside the button (for Buttons) or inside the anchor tag a (for ButtonLinks).

However, since the data-gtm-id prop needed for our Google Analytics tracking gets set on the button or anchor tag a (and not on the span nested inside them), the tracking of the user click will not work.

What we do instead is to make the span non-clickable so that the click will happen on the button or anchor tag a instead, which is achieved by setting pointer-events: none on the elements inside the button.

PR Checklist

Versioning

Just a patch bump (v17.10.0 -> v17.0.1) as this is not really a new feature. However, I would not bet my family on the fact that this doesn't break anything for clients... 😅 So I appreciate any input.