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

Removed remember popup properties setting #4367

Merged
merged 1 commit into from
Oct 7, 2020

Conversation

panoreak
Copy link
Contributor

@panoreak panoreak commented Oct 2, 2020

What is it?

  • Bug fix (user facing)
  • Feature (user facing)
  • Code base improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

  • Removed "Remember popup properties" setting in video and audio settings. The app now remembers the size and position of the video player popup by default.

Fixes the following issue(s)

-Issue #4329

Testing apk

removeRememberPopupPropertiesApk.zip

Agreement

@opusforlife2
Copy link
Collaborator

Tested. Works! Thank you!

Stypox
Stypox previously approved these changes Oct 6, 2020
@Stypox
Copy link
Member

Stypox commented Oct 6, 2020

Could you rebase? Then this is ready to merge, thank you for your contribution!

@panoreak
Copy link
Contributor Author

panoreak commented Oct 6, 2020

Could you rebase? Then this is ready to merge, thank you for your contribution!

Yes! I'm on it 👍

@panoreak panoreak force-pushed the remove-remember-popup-properties branch from ff25100 to 314615b Compare October 6, 2020 22:33
@Stypox Stypox merged commit bc342b9 into TeamNewPipe:dev Oct 7, 2020
This was referenced Nov 10, 2020
@DI555
Copy link

DI555 commented Nov 11, 2020

why you're deleting the choice of using options!
the option should be workable!
who give you the right to choose for others ???
personally, i, and many others should have the option to not save popup positions!!!

@opusforlife2
Copy link
Collaborator

Don't double post, or as in your case, triple post, because it achieves nothing except spamming people with notifications.

why you're deleting the choice of using options!

Because it was discussed and looked like a pointless option. The setting was actually introduced because a developer added the functionality to remember popup size and position. When you install Newpipe, it is on by default. Most people don't even seem to know it exists. And since it is the default behaviour, it was considered needless to have the toggle. You can just move the popup and resize it however you want within seconds.

who give you the right to choose for others ???

Lol, that is the definition of developing an app. You make choices for yourself and for all users of that app. If you want to gain that right, become a developer or join a development team.

personally, i, and many others should have the option to not save popup positions!!!

Now we come to the actual point, which is the only sentence you needed to type: What benefit do you get from the app not remembering popup position and size?

@DI555
Copy link

DI555 commented Nov 12, 2020

i might the one of some/many (as seems as were thumbups)... who used exactly this option.
logically i did this because the reason -
i may change popup size and position in time of using NP , but just doesn't want to get it remembered.., just was little cute option that don't want to lose.
and after all would suggest a little co-options 'popup default size' and 'popup default position' just to use with that i discribed upper!

the benefit is in that - i may variaty changing position and size during the session , but want it to be starting from one position and size what i want!
imho this concept with using optionable trigger switch to on/off is giving much more degrees of freedom!

and of course my appologize for @panoreak , huge work is done...
but things're always doing the way you think nobody use it, and suddenly appears some dudes that are using it for years((..
so i wanted to say that some thumbuped dudes and i will feel the lack of this option if it'll be cut out the app((..

@opusforlife2
Copy link
Collaborator

So if you would like it to start from a certain position the next time, you could place it in that position and then close the popup? Having 3 separate options for a niche feature is too much.

Also, when you regularly use the popup feature, eventually you find out a favourite size and position, which lets you multitask in the best manner. I don't imagine many people would deviate from it much in everyday use.

@DI555
Copy link

DI555 commented Nov 12, 2020

but, lets lalk without needness of it fornow...
it is an option yet, switchable on and off. guyes who doing this - you may use switch ON of this option, let us (others) using the swither in position OFF !!! Why don't you, hu ???

Have you made a vote for/against of this option before going to throw it away ???
Why did you decide just in talk with each other in irc chat that this option won't be needed for others ???!!! Some thumbups give me the right to talk about that..

@opusforlife2
Copy link
Collaborator

Have you made a vote for/against of this option before going to throw it away ??? Why did you decide just in talk with each other in irc chat that this option won't be needed for others ???!!!

We have several users in our IRC channel. No one objected to removing the setting. Furthermore, I opened an issue about it on Github (#4329) where, again, no one objected to it. So when a contributor came forward and asked to send a PR, we gave the go ahead.

Basically, you're the first person who has raised an objection to the removal, so we had no reason to believe until now that anyone wanted the setting.

Some thumbups give me the right to talk about that..

You don't gain special rights because of thumbs up or anything like that. Where did you get that idea? Simply by being a user of the app, you're more than welcome to talk about your problems. That's what the bug tracker is for. ;)

but, lets lalk without needness of it fornow...

Why? An option that no one needs is additional clutter. We should definitely keep 'need' in mind when we're talking about it.

@DI555
Copy link

DI555 commented Nov 12, 2020

i'm a bit in that situation, but it's a pity that i can't change it((.
but nevertheless ;), remember what the big g is doing and did with appearence of adress line??? yes, they just say the full line of adress isn't needed(( and they even didn't bring the switch to make the choice((

the other way, MoFo, if they're doing a new feature - they almost always making a switch for that in about:config!

so, if you decide eliminate an option in ui and/or symplify the options,- please try to make a switch for that in smth.analogue the mozilla's about:config page!
What do you think about that? ..there are a lots of parameters that may be tuned more gently, timings for examples, or some others...

@opusforlife2
Copy link
Collaborator

I agree with you about an about:config-like menu in settings. See my comment here: #4038 (comment)

@Stypox
Copy link
Member

Stypox commented Nov 12, 2020

Yeah, I also proposed an advanced-mode menu with miscellaneous niche options

@Stypox
Copy link
Member

Stypox commented Nov 15, 2020

This was reverted since some users (@DI555) turned out to be using this setting. A better solution is needed (like I said above, a miscellaneous/niche settings menu), but for now this won't be included

@DI555
Copy link

DI555 commented Nov 17, 2020

wow , @Stypox , great thanks for that!
i hope at least hiding this option into 'about:config'/'tech.config' will be the right way!

and i also have some solution about how this option should logically work rightly.
So, when 'remember popup properties' is 'on' , - all must work like it shoud, - you may choose your favorite size&position !!!
And after that if turn 'remember popup properties' to 'off' , - NP should remember your previousely chosen size&position as default size&position !!!
So could be possible always do popup start from your chosen size&position if even after you broke it or played with it during the viewing ;)

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.

Remove "Remember popup properties" toggle and make it the default behaviour
4 participants