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 2024: Top 5 Podcasts List #2340

Merged
merged 6 commits into from
Oct 28, 2024
Merged

Conversation

bjtitus
Copy link
Contributor

@bjtitus bjtitus commented Oct 26, 2024

📘 Part of: #2250

Completes #2317

To test

  • Enable the endOfYear2024 feature flag
  • Restart the app
  • Visit Profile
  • Tap the Playback 2024 Cell
  • Tap through until you get to this screen
  • Verify that the list is shown and accurate

Checklist

  • I have considered if this change warrants user-facing release notes and have added them to CHANGELOG.md if necessary.
  • I have considered adding unit tests for my changes.
  • I have updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

@dangermattic
Copy link
Collaborator

dangermattic commented Oct 26, 2024

1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@bjtitus bjtitus added the [Project] End of Year 2024 End of Year project for 2024 label Oct 26, 2024
@bjtitus bjtitus force-pushed the bjtitus/playback-2024/top-5 branch from e3168bb to 0200de7 Compare October 26, 2024 02:24
@bjtitus bjtitus marked this pull request as ready for review October 26, 2024 02:24
@bjtitus bjtitus requested a review from a team as a code owner October 26, 2024 02:24
@bjtitus bjtitus requested review from danielebogo and removed request for a team October 26, 2024 02:24
@bjtitus bjtitus force-pushed the bjtitus/playback-2024/top-5 branch from ee403f4 to bedd9f7 Compare October 27, 2024 21:33
Copy link
Contributor

@danielebogo danielebogo left a comment

Choose a reason for hiding this comment

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

I'm able to see the screen.
I'm going to approve it but I have 2 questions though that can be addressed on a different PR:
• Checking the design file, the animation should have a small bounce at the end. This seems a bit too linear
• Im struggling to navigate back between the screen. It depends on where I tap, some parts is unresponsive. Did you notice it?

@bjtitus
Copy link
Contributor Author

bjtitus commented Oct 28, 2024

Checking the design file, the animation should have a small bounce at the end. This seems a bit too linear

Woops! I meant this to be a spring. I've updated it and will probably circle back to triple check these values later this week. It looks pretty close to me as it is now.

Im struggling to navigate back between the screen. It depends on where I tap, some parts is unresponsive. Did you notice it?

Yeah, the ScrollView is affecting the navigation gesture handling. I'm looking into some better ways to handle this but I think this is the only view with the scrolling. For now, I've added a check for the smaller device sizes as a stop gap for now to disable the scroll view on the larger devices.

@bjtitus bjtitus merged commit 4ff64b3 into trunk Oct 28, 2024
4 of 6 checks passed
@bjtitus bjtitus deleted the bjtitus/playback-2024/top-5 branch October 28, 2024 17:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Project] End of Year 2024 End of Year project for 2024
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants