Long Press Playback Effect Turns Custom Effect On #1150

CalebKL · 2023-07-04T19:52:35Z

Description

This PR sets up custom playback effects when you long press the Playback effect Icon.

Fixes # @CookiedCodes niggles

Testing Instructions

Tap on the Discover tab
Tap on any random podcast
Long Press on the Playback effect Icon

Screenshots or Screencast

playback.mp4

Checklist

[X ] If this is a user-facing change, I have added an entry in CHANGELOG.md
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 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

…and toggle notifications

ashiagr · 2023-07-10T09:27:53Z

👋

I just gave it a brief run and the override does seem like a useful feature!

The shortcut to enable custom effects can remain but we were thinking if we could replace the snack bar like view with a toggle to switch between local and global podcast settings. Maybe it can remain visible all the time and be set to off by default.

/cc @david-gonzalez-a8c

CookieyedCodes · 2023-07-10T09:57:19Z

@ashiagr I was debating this as I was mocking it up and kind of came to the conclusion that a switch with a global/home icon in the top right would add too much visual clutter for this UI popup in my view but I'm not against it if it's just better 😅

Regarding what was implemented, I did find it a bit weird that it not popping up would not be possible but I suppose that's a problem with dealing with long press, after a day I was thinking why wouldn't a NAND gate not work tho,
As in IF info notification is turned on && the playback fx panel is not visible THEN popup the playback fx dialogue

That should make it quite elegant in my view, if that's possible correct me if I am wrong/it's not possible tho 🤔

Regarding your suggestion just to clarify are you suggesting a link (that would popup the playback fx) or that the notification has the toggle 🤔

ashiagr · 2023-07-10T10:16:19Z

Sorry that it was not clear. I meant this view on the playback effect dialog and if we can convert it to a switch that will allow us to toggle between local/podcast effects:

Regarding your suggestion just to clarify are you suggesting a link (that would popup the playback fx) or that the notification has the toggle 🤔

Oh no, notification is fine. No changes to that.

ashiagr · 2023-07-10T10:22:17Z

Regarding what was implemented, I did find it a bit weird that it not popping up would not be possible but I suppose that's a problem with dealing with long press, after a day I was thinking why wouldn't a NAND gate not work tho,
As in IF info notification is turned on && the playback fx panel is not visible THEN popup the playback fx dialogue

That should make it quite elegant in my view, if that's possible correct me if I am wrong/it's not possible tho 🤔

I didn't notice this behavior earlier. I think what you suggested should work but I haven't tried it. @Mzazi25 have you tried it already and come across any issues?

CookieyedCodes · 2023-07-10T10:50:12Z

Sorry that it was not clear. I meant this view on the playback effect dialog and if we can convert it to a switch that will allow us to toggle between local/podcast effects:

Regarding your suggestion just to clarify are you suggesting a link (that would popup the playback fx) or that the notification has the toggle 🤔

Oh no, notification is fine. No changes to that.

Ah I got you, I think it's not a bad solution I think the only problem is that people would have to read to distinguish between local vs Global and might cause some mistap issues hence my recent thought on the icons 🤔🤔

ashiagr · 2023-07-10T10:54:49Z

people would have to read to distinguish between local vs Global and might cause some mistap issues hence my recent thought on the icons

Oh, I see. Let's try out toggling through icons then 👍

CalebKL · 2023-07-10T11:03:10Z

@ashiagr I tried but since we are using a snackbar to toast the message, we can't adjust the position of the snackbar to the screen, hence it would not pop up above the "Custom Effects Applied"
I guess that was the only challenge

mchowning · 2023-10-18T18:27:32Z

👋 @Mzazi25 and @ashiagr

Just checking in on the status of this PR. Is there anything specific that needs to be done to move this forward?

CalebKL · 2023-10-18T18:39:22Z

👋 @Mzazi25 and @ashiagr

Just checking in on the status of this PR. Is there anything specific that needs to be done to move this forward?

I believe for this feature, there were some discussions going on, if it were a priority then. Give me a green light and I will work on it ASAP
@ashiagr @mchowning lmk what you suggest

CookieyedCodes · 2023-10-18T19:36:20Z

I was going to do a design like somewhat soonish -maby tomorrow- because I like the dial what Spotify did, I was also going to include a toggle for local & global playback FX 😉

CookieyedCodes · 2023-10-24T14:29:36Z

A new design for a speed settings, its a dial UI with preset manager -long press the UI number- as you swipe left and right the numbers to indicate location, the ability to toggle between global and local settings is also added to this UI, Lightly inspired by Spotify's new dial UI & minimalist radio tuning UI. Also unlocks 5x Speed because some people wants that as an option and would be less cumbersome with this UI (Local FX under podcast settings doesn't change UI) Any thoughts @Mzazi25 @ashiagr @mchowning @geekygecko @david-gonzalez-a8c

david-gonzalez-a8c · 2023-12-22T17:20:03Z

Hey there, @CookieyedCodes

Thank you for your work on this, nice job! Here are my thoughts on the designs:

I personally think that the dial UI adds a lot of complexity to this screen, and I kinda like how in the current version we have a balance in the three settings with the icon, then the name of the setting, then the setting itself. I think I'd like to keep that so I'd consider a couple alternatives:

Use the dial UI in a subsequent screen, keeping the current screen more simple, but I'm not sure about the double modal:

Change the behaviour of the current screen so tapping on the selected speed displays a menu (I'm showing iOS here, as my design files are for iOS, but an equivalent could be used for Android, like this component). This might be cleaner and more practical than the slider, and you still have the -/+ buttons to fine tune the speed if necessary.

What do you think?

CookieyedCodes · 2023-12-22T18:58:56Z

@david-gonzalez-a8c

The main reason I put it above was to prevent accidental miss touches but I wouldn't be against a seperate UI popup that could accessed from a long press of the number 🤔
also I designed it a tad funky to look not exactly like Spotify 😅(just to be clear long pressing on the diall saves the current time to the preset but my preference would be 1)

Any thoughts on the local/global toggle 🤔

CookieyedCodes · 2023-12-22T23:30:03Z

Gave a whole redesign of this screen more thought

mukul-scogo · 2024-01-01T09:56:47Z

...ures/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/PlayerViewModel.kt

@@ -395,6 +396,28 @@ class PlayerViewModel @Inject constructor(
        }
    }

+    fun toggleNotifications(context: Context) {
+        Toast.makeText(context, LR.string.player_effects_custom_effects_applied, Toast.LENGTH_SHORT).show()


Can we remove the UI related components like context from ViewModel and move to UI/fragment? We should not use context in viewmodel except UIs because it makes the testing harder.

mukul-scogo · 2024-01-01T10:07:40Z

...ures/player/src/main/java/au/com/shiftyjelly/pocketcasts/player/viewmodel/PlayerViewModel.kt

+        Toast.makeText(context, LR.string.player_effects_custom_effects_applied, Toast.LENGTH_SHORT).show()
+    }
+
+    fun updateOverrideGlobalEffects(override: Boolean) {


Here, override looks like a pre-defined naming convention. Can we use self-explanatory name?

CalebKL added 4 commits July 4, 2023 22:18

Added function to handle Long Press listener

ace44fa

updated PlayerViewModel with the function to override global effects …

d089ece

…and toggle notifications

updated PlayerHeaderFragment to handle LongPress on the playback effect

f405dd2

Merge branch 'main' into long-press-playback-effect

23edad0

CalebKL requested a review from a team as a code owner July 4, 2023 19:52

updated CHANGELOG.md with Long Press Feature on playback effect

a78a593

CookieyedCodes mentioned this pull request Dec 23, 2023

Increase limit on playback speed to 5.0 #1645

Merged

9 tasks

mukul-scogo reviewed Jan 1, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Long Press Playback Effect Turns Custom Effect On #1150

Long Press Playback Effect Turns Custom Effect On #1150

CalebKL commented Jul 4, 2023

ashiagr commented Jul 10, 2023

CookieyedCodes commented Jul 10, 2023 •

edited

Loading

ashiagr commented Jul 10, 2023

ashiagr commented Jul 10, 2023

CookieyedCodes commented Jul 10, 2023

ashiagr commented Jul 10, 2023

CalebKL commented Jul 10, 2023

mchowning commented Oct 18, 2023

CalebKL commented Oct 18, 2023

CookieyedCodes commented Oct 18, 2023

CookieyedCodes commented Oct 24, 2023 •

edited

Loading

david-gonzalez-a8c commented Dec 22, 2023

CookieyedCodes commented Dec 22, 2023

CookieyedCodes commented Dec 22, 2023

mukul-scogo Jan 1, 2024

mukul-scogo Jan 1, 2024

Long Press Playback Effect Turns Custom Effect On #1150

Are you sure you want to change the base?

Long Press Playback Effect Turns Custom Effect On #1150

Conversation

CalebKL commented Jul 4, 2023

Description

Testing Instructions

Screenshots or Screencast

Checklist

I have tested any UI changes...

ashiagr commented Jul 10, 2023

CookieyedCodes commented Jul 10, 2023 • edited Loading

ashiagr commented Jul 10, 2023

ashiagr commented Jul 10, 2023

CookieyedCodes commented Jul 10, 2023

ashiagr commented Jul 10, 2023

CalebKL commented Jul 10, 2023

mchowning commented Oct 18, 2023

CalebKL commented Oct 18, 2023

CookieyedCodes commented Oct 18, 2023

CookieyedCodes commented Oct 24, 2023 • edited Loading

david-gonzalez-a8c commented Dec 22, 2023

CookieyedCodes commented Dec 22, 2023

CookieyedCodes commented Dec 22, 2023

mukul-scogo Jan 1, 2024

Choose a reason for hiding this comment

mukul-scogo Jan 1, 2024

Choose a reason for hiding this comment

CookieyedCodes commented Jul 10, 2023 •

edited

Loading

CookieyedCodes commented Oct 24, 2023 •

edited

Loading