Analytics: Add REACH_RESULT_END event #2454
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
openverse-bot
added
🟨 priority: medium
|
Size Change: +2.83 kB (0%) Total Size: 843 kB
ℹ️ View Unchanged
|
8 tasks
obulat
force-pushed
the
add/REACH_RESULT_END_event
branch
from
afb0f8f to
ac40905
Compare
zackkrida
reviewed
Jun 21, 2023
zackkrida
reviewed
Jun 21, 2023
obulat
force-pushed
the
add/REACH_RESULT_END_event
branch
from
ac40905 to
87e8ae7
Compare
|
Based on the medium urgency of this PR, the following reviewers are being gently reminded to review this PR: @AetherUnbound 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. @obulat, if this PR is not ready for a review, please draft it to prevent reviewers from getting further unnecessary pings. Footnotes
|
sarayourfriend
approved these changes
Jun 27, 2023
AetherUnbound
approved these changes
Jun 27, 2023
|
I made two small changes here:
|
|
Nice catches! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fixes
Fixes #1072 by @dhruvkb
Description
The main goal of this PR was to add the
REACH_RESULT_ENDevent when the user scrolls down the search results page and sees the "Load More" button.To add this event, I used the
useElementVisibilitycomposable from Vueuese. However, because the page was not scrolled to top when the user changes the search type, when the user opened a second page of images, for example, and then switched search type, theREACH_RESULT_ENDevent would be sent right after the "Load More" button was clicked, before the next page was even loaded.This is why I also added scroll to top functionality to the search result page so now when the user changes the search type, the results page scrolls to top.
This change surfaced a problem where the search type changed, and there were still results from the previous search type, and the change of the media and search title/content links was visibly inconsistent after the page scrolled to top. To fix this, I added
clearMediacall to the function that changes the search type so that the page displays a loading skeleton once it scrolls up. For all content page, when the fetchState'sisFetchingwas false before the fetching started, this wouldn't show the loading skeleton, and the external search form was visible right below the content links. This is why I had to update the condition for showing the loading skeleton to include!hasStarted.To add the
resultPageto the event payload, I updated themediaStoreto keep track of the current page. @zackkrida, this might be useful for #2449 (I found a relatively simple way of adding the page, without major changes to the media and search stores)I also did some cleanup in the
search.vue: moved thefetchMediaandrouteWatcherinside thesetupfunction, and usedstoreToRefsto simplify the store values.All of this is to say that seemingly small addition required a lot of changes in the code, this is why the PR is large.
Testing Instructions
Checklist
Update index.md).main) or a parent feature branch.Developer Certificate of Origin
Developer Certificate of Origin