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

Playback 2023 - Top five podcasts story #1481

Merged
merged 15 commits into from
Oct 19, 2023
Merged

Playback 2023 - Top five podcasts story #1481

merged 15 commits into from
Oct 19, 2023

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Oct 18, 2023

📘 Part of: #1463 🎨 Designs: lH66LwxxgG8btQ8NrM0ldx-fi-1599_20385

Description

Adds the 2023 visual for the Top Podcasts Story.

Testing Instructions

  1. Make sure you're logged in to an account that has a few episodes listened
  2. Go to Profile
  3. Tap the EOY image
  4. Skip to the fourth story
  5. ✅ Check that the story looks good
  6. Share this story
  7. ✅ The story image should look good

Screenshots or Screencast

Pixel 7
Nexus 9 Tablet

Checklist

  • If this is a user-facing change, I have added an entry in CHANGELOG.md
  • Ensure the linter passes (./gradlew spotlessApply to automatically apply formatting/linting)
  • 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

@ashiagr ashiagr requested a review from a team as a code owner October 18, 2023 08:11
@ashiagr ashiagr mentioned this pull request Oct 18, 2023
25 tasks
@CookieyedCodes
Copy link

Still think it should be more then a top 5 😅

Base automatically changed from task/1463-top-podcast to main October 18, 2023 14:53
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.

This looks good. The only minor issue I saw is that the background highlight effect seems a bit light and might make it more difficult for some people to read the subtitle text since there's not much contrast. Not a blocker though.

image

@ashiagr
Copy link
Contributor Author

ashiagr commented Oct 19, 2023

Still think it should be more then a top 5 😅

Thanks for the suggestion, @CookieyedCodes! It has been communicated to the team. While it might be difficult to fit 10-15 top podcasts on this screen, we're considering showing them in the Stats section.

@ashiagr
Copy link
Contributor Author

ashiagr commented Oct 19, 2023

This looks good. The only minor issue I saw is that the background highlight effect seems a bit light and might make it more difficult for some people to read the subtitle text since there's not much contrast. Not a blocker though.

I realized that in my next PR where I rotated the blurred image (037f068) to get more contrast behind the text. I'll update the background for this story as well there.

# Conflicts:
#	modules/features/endofyear/src/main/java/au/com/shiftyjelly/pocketcasts/endofyear/views/stories/StoryTopPodcastView.kt
@ashiagr ashiagr enabled auto-merge (squash) October 19, 2023 04:49
@CookieyedCodes
Copy link

@ashiagr fair enough, I only request it because your podcast theamed end of year should be better then Spotifys 😅, more stats would allways be welcome 😁

@ashiagr ashiagr merged commit d5570e6 into main Oct 19, 2023
@ashiagr ashiagr deleted the task/1463-top-5-podcast branch October 19, 2023 05:16
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.

3 participants