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

Edge-to-edge dialog support #3414

Open
wants to merge 8 commits into
base: task/android-15
Choose a base branch
from

Conversation

geekygecko
Copy link
Member

@geekygecko geekygecko commented Jan 8, 2025

Description

This pull request aims to improve the dialog styling. Switching to Android 15 caused issues with the padding and colours, especially regarding the navigation bar.

The latest version might look worse on the older SDK 30 and 24 but it look a lot better on the higher SDKs when using the dialog. The content that slides behind the navigation bar handle isn't a solid navigation bar colour. For example before the change:

Screen.Recording.2025-01-10.at.3.18.06.pm.mov

After:

Screen.Recording.2025-01-10.at.3.19.26.pm.mov

This change took a while, so it doesn't take even longer I haven't focused on the landscape version. I will do that in a future PR if we have time to spend on it.

Testing Instructions

Try different bottom sheet dialogs in the app. And verify the content is displayed correctly with the navigation bar.

  • Podcast grid options: go to the Podcast tab, three dots in the toolbar.
  • Filter sort by: go to the Filters tab, select a filter, three dots in the toolbar, Sort By.
  • Clear Up Next: add some episodes to your Up Next, go to the Up Next tab, clear Up Next button in the toolbar.
  • File options: go to the Profile tab, tap Files, tap a user file.
  • Champion: login as a lifetime user, go to the Profile tab, tap account, tap Pocket Casts Champion.
  • Full screen player: open the full screen player, tap effects, tap sleep timer, tap the shelf overflow button.

Screenshots

Filter sort by dialog

After

Screenshot 2025-01-10 at 2 31 20 pm Screenshot 2025-01-10 at 2 32 10 pm

Before (main)

Screenshot 2025-01-10 at 3 08 03 pm

Podcast grid options dialog

After

Screenshot 2025-01-10 at 2 33 43 pm Screenshot 2025-01-09 at 9 10 49 am

Before (main)

Screenshot 2025-01-10 at 3 08 16 pm

Clear Up Next dialog

After

Screenshot 2025-01-10 at 2 56 21 pm Screenshot 2025-01-10 at 2 55 35 pm

Before (main)

main-clear-up-next

User file dialog

After
Screenshot 2025-01-10 at 2 57 57 pm
Screenshot 2025-01-10 at 2 58 59 pm

Before (main)
Screenshot 2025-01-10 at 3 08 54 pm

Pocket Casts champion

After

Screenshot 2025-01-09 at 9 07 00 am Screenshot 2025-01-09 at 9 08 25 am

Player - Long press next episode

Screenshot 2025-01-10 at 2 59 59 pm

Before (main)

Screenshot 2025-01-10 at 3 07 30 pm

Player - Playback effects

The extra bottom padding will go once we remove the old custom playback speed settings.

Screenshot 2025-01-10 at 3 01 00 pm

Before (main)

Screenshot 2025-01-10 at 3 07 05 pm

Player - Sleep timers

Screenshot 2025-01-10 at 3 01 37 pm

Before (main)

Screenshot 2025-01-10 at 3 07 16 pm

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 updated (or requested that someone edit) the spreadsheet to reflect any new or changed analytics.

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

@dangermattic
Copy link
Collaborator

dangermattic commented Jan 8, 2025

1 Error
🚫 PR requires a [Type] label and either a [Area] or [Project] label.
1 Warning
⚠️ PR is not assigned to a milestone.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Jan 8, 2025

📲 You can test the changes from this Pull Request in 📱 Mobile by scanning the QR code below to install the corresponding build.
App Name 📱 Mobile
Build TypedebugProd
Commitabc90b4
Direct Downloadpocketcasts-app-prototype-build-pr3414-abc90b4.apk
📲 You can test the changes from this Pull Request in 🚗 Automotive by scanning the QR code below to install the corresponding build.
App Name 🚗 Automotive
Build TypedebugProd
Commitabc90b4
Direct Downloadpocketcasts-automotive-prototype-build-pr3414-abc90b4.apk
📲 You can test the changes from this Pull Request in ⌚ Wear by scanning the QR code below to install the corresponding build.
App Name ⌚ Wear
Build TypedebugProd
Commitabc90b4
Direct Downloadpocketcasts-wear-prototype-build-pr3414-abc90b4.apk

@@ -197,6 +197,7 @@
<item name="isDark">false</item>
<item name="isImageTintEnabled">false</item>
<item name="imageTintThemeCacheTag">light</item>
<item name="enableEdgeToEdge">true</item>
Copy link
Member Author

Choose a reason for hiding this comment

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

The BottomSheetDialog material component uses this to know it should draw to the edge which fixes the navigation bar being a different colour to the background.

@geekygecko geekygecko force-pushed the task/edge-to-edge-dialog branch from 4af81bf to af41c16 Compare January 10, 2025 00:59
@geekygecko geekygecko marked this pull request as ready for review January 10, 2025 05:08
@geekygecko geekygecko requested a review from a team as a code owner January 10, 2025 05:08
@geekygecko geekygecko requested review from MiSikora and removed request for a team January 10, 2025 05:08
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.

3 participants