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

Search history - Merge feature branch #784

Merged
merged 55 commits into from
Feb 17, 2023
Merged

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Feb 17, 2023

Closes #752

Description

This merges "Search history" feature branch to main.

Testing Instructions

All PRs in #752 were reviewed and tested before merging into the feature branch. Smoke test the feature to see if everything's working after merging conflict resolutions.

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • I have considered whether it makes sense to add tests for my changes
  • All strings that need to be localized are in modules/services/localization/src/main/res/values/strings.xml
  • Any jetpack compose components I added or changed are covered by compose previews

I have tested any UI changes...

  • with different themes
  • with a landscape orientation
  • with the device set to have a large display and font size
  • for accessibility with TalkBack

Added separator explicitly removing space after comma.
…-all-and-navigation

Search history: Navigation and Clear All
Set up end of year modal bottom sheet when user is logged in only when eligible. Otherwise a blank compose modal sheet gets added that can take up focus and cause issues with talkback.
If podcastIds is empty, list should be empty after splitting by delimiter
ashiagr and others added 21 commits February 14, 2023 17:03
Max history count is added so that search history table doesn't grow too large
…y-analytics

# Conflicts:
#	modules/services/analytics/src/main/java/au/com/shiftyjelly/pocketcasts/analytics/AnalyticsEvent.kt
@ashiagr ashiagr requested a review from a team as a code owner February 17, 2023 06:32
@ashiagr ashiagr added this to the 7.33 milestone Feb 17, 2023
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.

Retested and everything is still working great! Wonderful work here @ashiagr !

One possible future improvement that came to my mind while I was testing... it might be nice if when you tap on a history item that item was moved to the top of the history list the next time you opened the list.

@mchowning mchowning merged commit f91e6d4 into main Feb 17, 2023
@mchowning mchowning deleted the feature/752-search-history branch February 17, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Search history
2 participants