Skip to content
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

Fix search results scroll position #847

Merged
merged 5 commits into from
Mar 30, 2023

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Mar 29, 2023

Description

This PR

  • Updates scroll position to the start of the list on a new search
  • Hides episodes on RSS podcast feed search

Warning
Targets release/7.35

Testing Instructions

Prerequisite: Enable search improvements (if you do not have valid google-services.json, return true from isFeatureFlagSearchImprovementsEnabled())

Scroll position

  1. Go to podcasts or discover tab search bar
  2. Search for a term
  3. Notice that search results are shown
  4. Scroll horizontally through the podcasts search results row so that the first visible item is not the first podcast in the search results
  5. Update search term
  6. ✅ Notice that the updated podcasts search results row scrolls to the first podcast in the results
  7. Scroll vertically so that the first visible episode is not the first episode in the search results
  8. Update search term
  9. ✅ Notice that the updated search results row scrolls to the first item in the vertical list
  10. Repeat step 4
  11. Tap on a visible podcast
  12. Navigate back to the search results screen
  13. ✅ Notice that the scroll position is retained

Screencast

device-2023-03-29-123916.mp4

Hide episodes from RSS podcast feed search

  1. Go to podcasts or discover tab search bar
  2. Search for a RSS podcast feed
  3. ✅ Notice that episodes are not shown and only the podcast for the RSS feed is shown in search results

Screencast

device-2023-03-29-124127.mp4

ashiagr added 3 commits March 29, 2023 12:18
This is done to prevent resetting podcast search results scroll position for the same search query.
Also, hide search history if rss podcast search result is present
@ashiagr ashiagr requested a review from a team as a code owner March 29, 2023 09:40
@ashiagr ashiagr changed the base branch from main to release/7.35 March 29, 2023 09:57
Copy link
Contributor

@mchowning mchowning left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and works well. 👍 Nice job!

ashiagr and others added 2 commits March 30, 2023 09:21
…tcasts/search/SearchInlineResultsPage.kt

Co-authored-by: Matt Chowning <[email protected]>
…tcasts/search/SearchViewModel.kt

Co-authored-by: Matt Chowning <[email protected]>
@ashiagr
Copy link
Contributor Author

ashiagr commented Mar 30, 2023

Thanks for the review, @mchowning! I found both of your explanatory comments useful and accepted them. 🙌

@ashiagr ashiagr merged commit 0c6a023 into release/7.35 Mar 30, 2023
@ashiagr ashiagr deleted the fix/search-results-scroll-position branch March 30, 2023 04:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants