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

PatchmanagerPage: Reorder pulley menu entries #272

Merged
merged 2 commits into from
Feb 23, 2022
Merged

PatchmanagerPage: Reorder pulley menu entries #272

merged 2 commits into from
Feb 23, 2022

Conversation

Olf0
Copy link
Contributor

@Olf0 Olf0 commented Feb 20, 2022

… "Settings" shall be below "About" and easier to reach, plus "Deactivate all Patches" the hardest to reach (i.e., topmost)

… "Settings" shall be below "About" and easier to reach, plus "Deactivate all Patches" the hardest to reach (i.e., topmost)
@Olf0
Copy link
Contributor Author

Olf0 commented Feb 20, 2022

I revisited this topic (literally, i.e., by looking at it closely in its current state) and the order just appears to be awkward: Why is the "Settings" entry above the "About" entry, which is much less often used; and why are they both above the dangerous "Deactivate all Patches" entry?
While we discussed that in issue #20, and I was willing to reconsider my former assessment in the light that "Deactivate all Patches" with a remorse timer is far less dangerous (if one realises quickly enough what is happening), I ultimately came to the conclusion that the current order just does not make any sense, as described above (i.e., it is more than just the position of the single "dangerous" entry).

Hence I "just did it": Reordered the topmost three entries so it does make sense (to me, at least).

@nephros
Copy link
Contributor

nephros commented Feb 21, 2022

… "Settings" shall be below "About" and easier to reach, plus "Deactivate all Patches" the hardest to reach (i.e., topmost)

I disagree with you there. In my experience, top menu entries are easiest to reach by acccident (maybe tied with the first/lowest entry when you somehow lose touch).
(They are also the easiest to reach on purpose, by vigurously flicking.)

About the general Pulley menu order, it's very common for in the SFOS app space to have About at the top, Settings next, and all others below those. We should follow that layout.

@Olf0
Copy link
Contributor Author

Olf0 commented Feb 21, 2022

… "Settings" shall be below "About" and easier to reach, plus "Deactivate all Patches" the hardest to reach (i.e., topmost)

I disagree with you there. In my experience, top menu entries are easiest to reach by acccident (maybe tied with the first/lowest entry when you somehow lose touch). (They are also the easiest to reach on purpose, by vigurously flicking.)

About the general Pulley menu order, it's very common for in the SFOS app space to have About at the top, Settings next, and all others below those. We should follow that layout.

Yes! Luckily we have converging opinions here (with issue #20 as a starting point), because I always perceived it as very awkward that Patchmanager has "Settings" as the topmost entry and "About" directly below it.

So the only point left where we do still not concur is the position of the "Deactivate all Patches" entry.
Some context, plus my considerations and experiences:

  • The pulley menu entry "Deactivate all Patches" is a relict from Patchmanager 2, because with PM 1 & 2 one must deactivate ("unapply" before PM 3.2.1) all Patches before upgrading SailfishOS else "hell will break loose"; seriously, SailfishOS installations were often broken beyond repair, because it was not feasible for users to determine which packages to reinstall to "heal" a specific installation: Factory-reset or flashing anew was the "easy way out", but a very tedious one.
  • Since PM 3 this pulley menu entry has become superfluous. This functionality is also available at the command line: patchmanager --unapply-all. And I also experienced recently how easily PM 3 enters "fail safe mode" (without even being guilty, but it sure cannot detect that), while having seen the code for "fail safe detection and mode" for the first time a few months ago.
    I believe it would be / have been a good maintainers task to eliminate it in PM ≥ 3.0.0, to save people from still performing this tedious back and forth of deactivating all Patches before upgrading SailfishOS and re-activating the non-conflicting ones after the upgrade ("because I have heard that this is necessary"), which has become a voodoo ceremony (i.e., black magic, which only works for the ones believing in it).
    The only thing I am unsure about how many people (and how loudly) will complain, because they cannot perform their voodoo ceremony any more (this goes a bit in line with Peter's point depicted via XKCD in issue [Usability issue] Please move the entry "Unapply all patches" in Patchmanager's top level pulley menu one or two positions further up, i.e. *above* the "About" entry #20), and if it is better to endure that or to keep offering this pulley menu entry. Personally I am fine with either way.
  • In my experience I slipped (i.e., lost control) most often somewhere in the middle of long pulley menus (i.e., with a lot of entries): Specifically for Patchmanager I did accidentally select menu entry "Unapply all Patches" (when it was still called that) at least three times, because it is nicely placed in the middle of a pulley menu with at least 4 entries. Which sent me right into hell to figure out, which combination of non-conflicting Patches (out of then installed ones) I had activated before; one could easily spent an hour with that when using PM 2 with >> 50 Patches installed. While this has become a lot easier to determine (possibly conflicting Patches are displayed in each Patch's details), I can tell from the recent "fail safe"-experiences that it still takes almost half an hour with >> 50 Patches installed (the first time; one becomes quicker when doing this repeatedly).
  • You, Peter, have alleviated the whole issue by attaching a remorse timer to this action, if one is quick enough to realise what happened, and if one is not fooled by recently reversed logic of confirming the action (by a swipe or tap on the little cross, which formerly cancelled the action).
  • No, "vigorously flicking" does not select the topmost entry: This lets the pulley fully stick out with some extra space atop the topmost menu entry and no entry selected.

Thus I am seriously asking either to "hide" this "dangerous" pulley menu entry where it does least harm (which is the topmost entry IMO) or to eliminate it (which likely is the proper solution).
I will gladly alter this PR to the latter solution, if this is your preference.

@nephros
Copy link
Contributor

nephros commented Feb 22, 2022

I have no objections to removing it competely, or add and option to show/hide it in the "Advanced" Settings.

Although I do find it useful sometimes and will problably patch it back in privately it it's removed ;)

About failure/panic mode:

I haven't looked into it, but I think that is activated whenever lipstick crashes for any reason.
I have also no idea yet what the "resolve failure" is supposed to do, it seems to do more or less nothing right now.

I would expect it to restore the configuration that was last saved*), of at least give to option to do that.

But these things are something to be discussed in a separate issue, not here. (#277, #278)

*) flashbacks of Windows last good configuration

@nephros
Copy link
Contributor

nephros commented Feb 22, 2022

I guess for this PR we have an agreement that reordering the Settings/About is what we'll do.

Please make a push doing that, and if you like comment out the "unapply all" Menu entry. If you do I will implement the Settings part on top later.

@Olf0
Copy link
Contributor Author

Olf0 commented Feb 23, 2022

Commit e140cfd disables the "Deactivate all Patches" pulley menu entry.

Please check that I have done that correctly, I have to look up the f**king QML syntax even for such simple things.
Side note: Did I already mention 😜, that I hate JavaScript and QML ☠️! It was invented to enable those "web-developers" inflicting harm at other places than web-pages (where is easy to suppress in a controlled manner by NoScript), in contrast to QML.

@nephros nephros merged commit d73c169 into master Feb 23, 2022
@nephros nephros deleted the Olf0-patch-2 branch February 23, 2022 07:24
nephros added a commit to nephros/patchmanager that referenced this pull request Feb 25, 2022
nephros added a commit to nephros/patchmanager that referenced this pull request Feb 25, 2022
nephros added a commit to nephros/patchmanager that referenced this pull request Feb 25, 2022
nephros added a commit that referenced this pull request Feb 26, 2022
* fixup! PatchmanagerPage: Reorder pulley menu entries (#272)

* Control Unapply All option through settings

* update Requires for Nemo.Configuration plugin

* fixup! Control Unapply All option through settings

* PatchManagerPage: Enhance comment

* SettingsPage: Enhance commment and new strings

* SettingsPage: Hope that "for" is a correct preposition

… after trying "at" and pondering about "on" (both "feel" plausible, but not like a perfect fit).

Co-authored-by: olf <[email protected]>
nephros pushed a commit to nephros/patchmanager that referenced this pull request Nov 26, 2024
otherwise the view.busy is undefined.
This had previously been handled differently but was overlookedin the
changes about unapply all.

See 573ce87 for the original handling, and PR sailfishos-patches#272.
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.

2 participants