Fix privacy opt-out button not clickable #3818
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
@yk-tuturu is attempting to deploy a commit to the modsbot's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
Thank you for your contribution! I am able to see your fix as intended — the toggle is set to "Opt-out" and disabled if matomo is not initialised.
While I think your solution fulfils the original comment made by an earlier maintainer, I think that adding a hook is unnecessary (and an incorrect usage of React hooks, see this comment).
I think the crux of this issue is if matomo is undefined, the opt-out toggle cannot be clicked, leading the user to believe that they cannot opt-out of the tracking, when in fact there is no tracking at all.
The current code that modifies the UI component actually already fixes this issue, and the addition of the hook is unnecessary. I think if we reverse the changes to the hook portion of this PR, it will LGTM.
| export function initializeMamoto() { | ||
| export function initializeMatomo() { |
There was a problem hiding this comment.
On line 24 or 25 of this same file, there's the same mispelling of Matomo, can we fix that as well?
| export function useMatomo() { | ||
| // need to use useState here or the matomo returned will always be undefined | ||
| const [matomoCopy, setMatomoCopy] = useState<Tracker | undefined>(undefined); | ||
| useEffect(() => { | ||
| setMatomoCopy(matomo); | ||
| }, [matomo]); | ||
|
|
||
| return matomoCopy; | ||
| } |
There was a problem hiding this comment.
In the following code:
useEffect(() => {
setMatomoCopy(matomo);
}, [matomo]);This effect is only ever called once (on component mount) since the matomo variable exists outside of the React context. This is reflected by the linter as well:
React Hook useEffect has an unnecessary dependency: 'matomo'. Either exclude it or remove the
dependency array. Outer scope values like 'matomo' aren't valid dependencies because mutating
them doesn't re-render the component.
ravern
changed the title
Context
closes #3254Implementation
following instructions detailed in issue #3254, implemented checks to see if matomo is initialized and disables the privacy toggle if blocked by an ad-blockerOther Information