-
Notifications
You must be signed in to change notification settings - Fork 212
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
Turn analytics composable into a plugin #3811
Conversation
This commit keeps the composable to reduce code churn and to prove that existing tests of the analytics work, proving the change works as expected
Full-stack documentation: https://docs.openverse.org/_preview/3811 Please note that GitHub pages takes a little time to deploy newly pushed code, if the links above don't work or you see old versions, wait 5 minutes and try again. You can check the GitHub pages deployment action list to see the current status of the deployments. |
Based on the medium urgency of this PR, the following reviewers are being gently reminded to review this PR: @zackkrida Excluding weekend1 days, this PR was ready for review 4 day(s) ago. PRs labelled with medium urgency are expected to be reviewed within 4 weekday(s)2. @sarayourfriend, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
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.
Works great! All of my local breakpoint, height, and width values are correct.
Co-authored-by: Zack Krida <[email protected]>
1f86352
to
c743833
Compare
Fixes
A pre-requisite for #3721
Description
Convert the
useAnalytics
composable into a plugin. I've chosen to take an approach where the composable still exists, but just acts as a thin wrapper around$sendCustomEvent
from the new plugin. This (a) reduces code churn and (b) allows us to confirm that the change behaves exactly as we expected the previous version to behave. All tests asserting against the analytics events continue to work without changes.Testing Instructions
All Playwright and unit tests should continue to pass without changes.
Furthermore, to confirm absolutely that the
breakpoint
,height
andwidth
variables still work as expected, run Plausible locally usingjust frontend/up
followed byjust frontend/init
to get the custom events loaded. Then visit frontend through the Nginx container at http://0.0.0.0:50290Perform a search and confirm that the three default props (listed above) match the expected values based on the viewport of your browser. Make multiple searches with different viewport sizes, especially making sure to trigger different breakpoints, and confirm that those events have the correct values.
Checklist
Update index.md
).main
) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin