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 scroll indicator behind player for long episode lists #1197

Conversation

rviljoen
Copy link
Contributor

@rviljoen rviljoen commented Nov 2, 2023

Fixes #1072

When scrolling long episode lists, the vertical scroll bar indicator no longer disappears behind the mini player when you get to the bottom of the list

To test

  1. Open current build and find long episode list (100+ should do)
  2. As you scroll to the bottom, notice that the vertical scroll indicator moves beyond the bottom of the episode list. If you scroll down far enough, it will disappear completely.

IMG_5171

  1. Build with the changes in this PR and repeat the steps above. Notice that the scroll indicator now stays above the mini player

image

  1. Repeat the same test with episode list in multi-select mode enabled

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.

@rviljoen rviljoen requested a review from a team as a code owner November 2, 2023 08:10
Copy link
Member

@leandroalonso leandroalonso left a comment

Choose a reason for hiding this comment

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

@rviljoen this is a step forward in fixing this issue, however when the mini player is not enabled this feels like a bug to me:

Simulator.Screen.Recording.-.iPhone.15.-.2023-11-23.at.13.36.17.mp4

For SwiftUI we created the MiniPlayerPadding view modifier.

I wonder if we can apply something similar here?

Ps.: we also have the same issue on Profile > Starred/Downloaded/Listening History. We don't necessarily need to tackle this here but it doesn't fix the issue in all places.

@rviljoen
Copy link
Contributor Author

@leandroalonso Good feedback, thanks. I'll take a look.

@emilylaguna
Copy link
Contributor

👋 If it helps, there is a common UITableView extension that applies the mini player padding in other areas in the app:

func applyInsetForMiniPlayer(additionalBottomInset: CGFloat = 0) {

@rviljoen rviljoen closed this Dec 7, 2023
@rviljoen rviljoen force-pushed the fix/1072-scroll-indicator-behind-player branch from 1b88883 to 91221b5 Compare December 7, 2023 13:48
@rviljoen
Copy link
Contributor Author

rviljoen commented Dec 7, 2023

I played around some more and it turns out that fixing this may require a bit more effort and testing than what it is worth for fixing a relatively small visual glitch. I have closed the PR for now and may relook at it in future.

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.

Bug: Scroll indicator behind player
3 participants