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

[Podcast Update Feed] Add Tooltip #3561

Merged
merged 9 commits into from
Feb 7, 2025
Merged

Conversation

mebarbosa
Copy link
Contributor

@mebarbosa mebarbosa commented Feb 6, 2025

Description

  • This adds the tooltip for podcast update feed
  • While I was testing I noticed that the tooltip is not being displayed in a very specific scenario:
    1. Small device
    2. The user did not dismiss the tooltip yet
    3. and open a podcast that does not follow
  • This is happening because we are aligning the tooltip with the more options button, which is not visible. I asked David about this here: p1738863222122779/1738285496.288959-slack-C088RDU8HAN
  • See: p1738782693924129/1738285496.288959-slack-C088RDU8HAN

Testing Instructions

Feature flag disabled

  1. Have the podcast update feed feature flag disabled
  2. Open an podcast
  3. You should not see the tooltip

Feature flag enabled

  1. Have the podcast update feed feature flag enabled
  2. Open an podcast
  3. If the more options button is visible, you should see the tooltip
  4. Tap out of the tooltip
  5. The tooltip should be closed
  6. Go to Discover
  7. Back to this podcast you have just opened
  8. You should see the tooltip again
  9. Dismiss the tooltip by tapping on close button
  10. Go to Discover
  11. Open the podcast again
  12. You should not see the tooltip

Screenshots or Screencast

Screen_recording_20250206_150940.webm

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

@mebarbosa mebarbosa added [Type] Enhancement Improve an existing feature. [Project] Podcast feed update Allow users to manually trigger a podcast feed update. labels Feb 6, 2025
@mebarbosa mebarbosa added this to the 7.83 milestone Feb 6, 2025
@mebarbosa mebarbosa requested a review from a team as a code owner February 6, 2025 18:24
@mebarbosa mebarbosa requested review from geekygecko and removed request for a team February 6, 2025 18:24
Comment on lines 69 to 75
<androidx.coordinatorlayout.widget.CoordinatorLayout
android:id="@+id/fullScreenDarkOverlayView"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:background="#80000000"
android:visibility="gone" />

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was the only way I found to display the tooltip overlay in front of the bottom navigation view. Similar what we are doing for snackbarFragment

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Feb 6, 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
Commit34a6e8f
Direct Downloadpocketcasts-app-prototype-build-pr3561-34a6e8f.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
Commit34a6e8f
Direct Downloadpocketcasts-automotive-prototype-build-pr3561-34a6e8f.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
Commit34a6e8f
Direct Downloadpocketcasts-wear-prototype-build-pr3561-34a6e8f.apk


private fun configureTooltip() {
lifecycleScope.launch {
delay(1.seconds) // Delay to wait the recyclerview to be configured
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added this delay because there is a race condition. Since we are aligning the tooltip with one recyclerview's item, I need to make sure this component has loaded. Using episodesRecyclerView?.doOnNextLayout was not enough

Copy link
Member

Choose a reason for hiding this comment

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

I don't think this delay works if the page takes a while to load.

delay.mov

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's fixed now. I had to mock low internet connection to be able to see this issue. See how it is now:

Screen_recording_20250207_123341.webm


private fun configureTooltip() {
lifecycleScope.launch {
delay(1.seconds) // Delay to wait the recyclerview to be configured
Copy link
Member

Choose a reason for hiding this comment

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

I don't think this delay works if the page takes a while to load.

delay.mov

app/src/main/res/layout/activity_main.xml Outdated Show resolved Hide resolved
@mebarbosa mebarbosa enabled auto-merge (squash) February 7, 2025 15:41
@mebarbosa mebarbosa disabled auto-merge February 7, 2025 15:44
@mebarbosa mebarbosa merged commit 4d67661 into main Feb 7, 2025
16 checks passed
@mebarbosa mebarbosa deleted the task/add-tooltip-for-podcast-feed branch February 7, 2025 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Project] Podcast feed update Allow users to manually trigger a podcast feed update. [Type] Enhancement Improve an existing feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants