search

ReVanced / revanced-patches

🧩 Patches for ReVanced
https://revanced.app
GNU General Public License v3.0
2.39k stars 276 forks source link

Toggle switches and default behaviours for settings are confusing #1970

Closed SodaWithoutSparkles closed 1 year ago

SodaWithoutSparkles commented 2 years ago

Type

Cosmetic

Bug description

When patching, some patches are included but does not enable by default in Revanced youtube. Examples include tablet mini-player and remember quality.

In Revanced YT settings, the toggle switches are confusing. Examples include "Remember quality" settings under misc. have to be turned off to remember quality, unlike what the title suggest.

Steps to reproduce

  1. Include all patches except debug, auto-repeat, wide search bar
  2. Patch YT 17.36.37 with manager 0.0.30
  3. Observe that the default behaviour is not as expected like what the bug description said

Relevant log output

N/A

Screenshots or videos

No response

Solution

Functions that are added using patches should be enabled by default unless they will significantly affect UX. Examples include:

But those which significantly affect UX like removing like/dislikes button should be not enabled

All toggles should be respecting what the title said, like the "remember video quality" toggle mentioned above. It should be toggled on to remember video quality All toggles under revanced settings > ads and layout should be configured to be "toggled on to remove something". Some examples are:

Ofc the button toggle state should be changed accordingly

Additional context

No response

Acknowledgements

oSumAtrIX commented 2 years ago

Give a suggestion for the remember selected quality toggle. Other patches defaults are intentionally set they way they are set, such as the tablet mini player toggle, because they heavily change the behavior of the client, if you enable patches you are suggested to revise them after applying.

SodaWithoutSparkles commented 2 years ago

I now understand the reason behind the default choices of enable/disable certain patches. In such case having an option to export settings would be better.

But that does not solve the "confusing toggle title" thing. Remember quality should be toggled on to enable the function, not off.

I dont think by default enabling remember quality is changing the behaviour heavily tho, but anyway, thats just a side quest.

I still thinks the wording of the toggle title should be changed such that "Toggle on everything means removing everything" or "Toggle on everything means show everything". Currently its all over the place which is not quite user friendly.

Edit: A pop-up on the first launch to remind users to check the settings would be nice.

oSumAtrIX commented 2 years ago

I was referring to tablet mini player, other toggles need to be normalized in behaviour, the remember quality toggle serves a different purpose. Toggle it on, and it will remember the current settings and never switch the quality even if you change it in the flyout, toggle it off and it will remember the quality until you change it in the flyout, basically semi persistent, which is why the toggle is named how it is named. Can you suggest a better name?

SodaWithoutSparkles commented 2 years ago

Toggle it on, and it will remember the current settings and never switch the quality even if you change it in the flyout, toggle it off and it will remember the quality until you change it in the flyout, basically semi persistent, which is why the toggle is named how it is named. Can you suggest a better name?

Oh I understand the reason behind the naming. If able, I would prefer the quality selection of the old vanced method, giving a menu to choose instead of having to switch networks and config. It would be even better if we could somehow prefer VP9 or not prefer AV01 codec, since some device does not support AV01 hardware decode.

A better method if we dont have the old vanced way would be leave the remember quality toggle under misc default off. When the user changes the quality, the toast notification pops up to tell users that it had remembered the selection, and advice user that it could be locked. The toggle could be called "Lock current remember quality preference" and have a description reminding users of the current preference, something like this:

Remembered quality for WiFi: 1080p. \n Go to Settings > misc to lock this preference

Lock current remember quality preference Switching quality [will/will not] affect the global preference. Currently [1080p] for WiFi and ... Click here to clear remember quality preference

But I still prefer the this way:

Default WiFi quality: 1080p Default cellular quality: 720p Preferred video codec: VP9

Sorry for misinterpreting your comment on tablet mini-player

sawankumarbundelkhandi commented 2 years ago

A better method if we dont have the old vanced way would be leave the remember quality toggle under misc default off. When the user changes the quality, the toast notification pops up to tell users that it had remembered the selection, and advice user that it could be locked. The toggle could be called "Lock current remember quality preference" and have a description reminding users of the current preference, something like this:

It's WIP here - https://github.com/revanced/revanced-patches/pull/723/files

SodaWithoutSparkles commented 2 years ago

For reference, the above PR was closed and moved here. https://github.com/revanced/revanced-patches/pull/826

SodaWithoutSparkles commented 2 years ago

@oSumAtrIX Maybe this normalization treatment should also be applied to the layout tab. Toggle on to modify default behaviour and vice versa. Including but not limited to:

The Remember quality selection settings has the right default settings, but the wording has not changed. I still thinks that "Lock current remember quality preference" would be better

thegravity98 commented 1 year ago

For " remember current video quality"

it should save user preference when the toggle is ON

it should not save user preference when the toggle is OFF.

Currently, the behaviour of the toggle is exactly opposite.

  1. Turn Off the toggle

  2. set the video quality (1080)

  3. There's a pop up that preference has been saved. (Changed default quality for WiFi to 1080)

  4. It saves my preference for next video

Actually this should occur when the toggle is turned ON. 🤦‍♂️

Should it remember video quality preference? Yes or No ?

Toggle ON - Yes

Toggle Off - No

SodaWithoutSparkles commented 1 year ago

it should save user preference when the toggle is ON

it should not save user preference when the toggle is OFF.

Thats why I said change it to "Lock current quality selection preference" would be better. It sounds like a lock to the config that would otherwise be changed everytime you change the quality.

That said, the old vanced quality selection menu is WIP, but I hope we could rank quality preference instead of setting one and let it fallback, as said in ReVanced/revanced-patches-template#1852.

oSumAtrIX commented 1 year ago

The toggle is not controlling what you say. It either locks or unlocks the current quality from being changed at all as explained in the switch description.

oSumAtrIX commented 1 year ago

@SodaWithoutSparkles I am still thinking about merging ReVanced/revanced-patches-template#1852 at all. It is a regression in UX. You have to visit the settings to select a quality which is the wrong place to begin with, while with the current one, the native quality selection works as intended: Select a quality - videos start playing in that quality. Change it to another, will start to play videos in the new quality. Want to remember the selected quality and only change it once for specific videos? Enable the switch for said behaviour. The only issue currently is that users which used Vanced design are used to it and are not introduced to this new design intuitively.

SodaWithoutSparkles commented 1 year ago

I think you are misunderstanding something here. What I mean in ReVanced/revanced-patches-template#1852 is add options in the existing quality selection screen. From only selecting 1080p60 now, to having 3 options for 1080p60, and if available another 3 for 1080p30. User dont need to go to settings to change the quality for a video, just have more options to choose.

Or you are talking about PR ReVanced/revanced-patches#826?

oSumAtrIX commented 1 year ago

@SodaWithoutSparkles off topic, I am responding in relation to this issue. Please open or use an issue dedicated for what you mean.

thegravity98 commented 1 year ago

I understood the behaviour now.

The word "current" is confusing.

It should be replaced with "default"

Case 1:

[ Modify Default Quality ]

Toggle is OFF Description should be "The current video quality will not be set as default for new video"

Toggle is ON Description should be "The current video quality will be set as default for new video"

Case 2:

[ Remember Default Quality ]

Toggle is ON Description should be "The current video quality will not be set as default for new video"

Toggle is OFF Description should be "The current video quality will be set as default for new video"

I prefer CASE 1 ...

SodaWithoutSparkles commented 1 year ago

okay, I would cut away the irrelevant sections and paste them to 909.

oSumAtrIX commented 1 year ago

@karl-pae Switch on means lock the quality/don't change it further more. The default behavior is the one which YouTube should be doing natively, which is simply to use the quality, which you selected.

thegravity98 commented 1 year ago

@oSumAtrIX

I understood completely. I agree when it's ON it is locked. That is Case 2.

I would request you to change the title & description a little bit.. if possible.

oSumAtrIX commented 1 year ago

Please suggest one.

thegravity98 commented 1 year ago

[ Remember Default Video Quality ]

Toggle is ON Description should be "The current video quality will not be set as default for new video"

Toggle is OFF Description should be "The current video quality will be set as default for new video"

Edit:

Also the pop up says

" Changing Default Wi-Fi Quality "

oSumAtrIX commented 1 year ago

Default is the wrong word as you highlighted it. So in the switch description it would be wrong too.

thegravity98 commented 1 year ago

I think, toggle for this is not a good UX it should be replaced with vanced settings.

Why ?

After fresh install of revanced YT app,

Now user want to lock 1080:

--- Vanced settings --- User don't need to play video just to lock quality.

Similar vanced setting is also used in Newpipe.

Vanced settings ::

Screenshot_2022-11-03-13-35-56-91_7a4090f09f6554852d748ee9fd6f40d3~2.jpg

Newpipe settings ::

Screenshot_2022-11-03-13-36-07-82_ee8807d2de22f1831269d80920e33a3b~3.jpg

oSumAtrIX commented 1 year ago

It is less often the case, that you want to change a locked quality. If you do it often, disable the lock once and „remember“ the quality.

thegravity98 commented 1 year ago

I don't have a problem with toggle but just to set one setting, the user has to go here and there.

Edit: Why should I play video just to set preferred quality ?

oSumAtrIX commented 1 year ago

The user would require to go to the settings in either way. As previously said, you can remember instead of locking if you change it often, as simple as that.

Meganium97 commented 1 year ago

I don't get how to set quality to "auto". Sometimes I want to save battery and other times I want to view a video in full quality, but I set it to not remember quality in misc settings, and it's now just stuck on 720p. No matter what I do, with the exception of toggling the misc setting and manually changing the stored quality setting, it always starts the video on 720p. I just don't understand.

oSumAtrIX commented 1 year ago

Disable the old quality layout patch and set it to auto in the flyout menu.

Meganium97 commented 1 year ago

Surprisingly doesn't work. I don't get the popup saying it universally changes quality to auto.

oSumAtrIX commented 1 year ago

https://user-images.githubusercontent.com/13122796/205517008-5e210d80-8940-4646-ac6e-c281b434625a.mp4

Does for me.

Meganium97 commented 1 year ago

I meant that selecting auto set it to auto but doesn't set every video to auto.

oSumAtrIX commented 1 year ago

Exclude the patch to remember the video quality.

SodaWithoutSparkles commented 1 year ago

@oSumAtrIX I hereby request a re-open of this issue to address some more confusing/inconsistent toggles.

Screenshot_2022-12-07-17-45-58-659-edit_app.revanced.android.youtube.jpg

oSumAtrIX commented 1 year ago

Open a new issue and only list every remaining switch which needs to be fixed.