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

Custom Playback Settings - Apply local/ global settings #3088

Merged
merged 11 commits into from
Oct 28, 2024

Conversation

ashiagr
Copy link
Contributor

@ashiagr ashiagr commented Oct 24, 2024

Description

This applies local/ global settings to the current playing episode using the new segemented bar control.

** Ignore local files for now

Part of #3042

Testing Instructions

Test Global Effects Settings

  1. Play an episode and open the Effects view
  2. Make sure the All Podcasts is selected
  3. Apply some changes
  4. Verify that selecting This podcast updates the view by changing the settings
  5. Verify that tapping back to All podcasts the previous settings are applied
  6. Verify the global settings are applied to all podcasts episode and that the selected tab is always All podcasts
  7. Navigate to the podcast page settings and verify that the local effects settings switch is off

Test Local Effects Settings

  1. Play an episode and open the Effects view
  2. Make sure the All Podcasts is selected
  3. Select This podcast tab
  4. Apply some changes
  5. Verify that selecting All podcasts updates the view by changing the settings
  6. Verify that tapping back to This podcast the previous settings are applied
  7. Verify that the settings are applied to this podcast only
  8. Verify that when opening the Effects view in this podcast, the tab selected is always This podcast
  9. Navigate to the podcast page settings and verify that the local effects settings switch is on (if you haven't subscribed to this podcast, make sure to subscribe to it)

Screenshots or Screencast

global_and_local_playback_effects.mp4

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 Oct 24, 2024

1 Warning
⚠️ Class PlaybackEffectsSettingsTab is missing tests, but unit-tests-exemption label was set to ignore this.

Generated by 🚫 Danger

@wpmobilebot
Copy link
Collaborator

wpmobilebot commented Oct 24, 2024

📲 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
Commitffa9f9a
Direct Downloadpocketcasts-app-prototype-build-pr3088-ffa9f9a.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
Commitffa9f9a
Direct Downloadpocketcasts-automotive-prototype-build-pr3088-ffa9f9a.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
Commitffa9f9a
Direct Downloadpocketcasts-wear-prototype-build-pr3088-ffa9f9a.apk

Comment on lines +190 to +196
if (FeatureFlag.isEnabled(Feature.CUSTOM_PLAYBACK_SETTINGS)) {
podcastDao.findByUuid(podcastUuid)?.let { localPodcast ->
podcast.copyPlaybackEffects(
sourcePodcast = localPodcast,
)
}
}
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'm copying local playback effects to the podcast here so that local playback effects are not lost while subscribing to it.

@ashiagr ashiagr marked this pull request as ready for review October 25, 2024 04:21
@ashiagr ashiagr requested a review from a team as a code owner October 25, 2024 04:21
@ashiagr ashiagr requested review from geekygecko and removed request for a team October 25, 2024 04:21
@CookieyedCodes
Copy link

CookieyedCodes commented Oct 25, 2024

Looking forward to this,
Just a reminder that files uses global and some tests might need to be done because unfortunately their is no local toggle for files,

Is their a chance with this project that settings sync only for playback fx be enabled? Obviously theirs risk I get that but if local settings get saved and synced this will save so much time on new devices & even if it mucks up its way less steps then it was previously 🤔

@ashiagr
Copy link
Contributor Author

ashiagr commented Oct 25, 2024

@CookieyedCodes 👋

Just a reminder that files uses global and some tests might need to be done because unfortunately their is no local toggle for files,

Thanks for the reminder!

Is their a chance with this project that settings sync only for playback fx be enabled? Obviously theirs risk I get that but if local settings get saved and synced this will save so much time on new devices & even if it mucks up its way less steps then it was previously 🤔

Syncing podcast-level effects settings is out-of-scope for this project. There could be updates in the future. 🤞

Base automatically changed from task/3042-add-ff-and-control to main October 28, 2024 05:08
# Conflicts:
#	modules/features/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/view/EffectsFragment.kt
#	modules/services/compose/src/main/java/au/com/shiftyjelly/pocketcasts/compose/components/SegmentedTabBar.kt
@ashiagr ashiagr enabled auto-merge (squash) October 28, 2024 05:17
@ashiagr ashiagr merged commit c8f1bd8 into main Oct 28, 2024
16 checks passed
@ashiagr ashiagr deleted the task/3042-local-global-logic branch October 28, 2024 05:45
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.

5 participants