Show list of things to be restarted on the "Restart Services" dialog #136

nephros · 2021-11-01T19:38:09Z

"Inspired by" Issue-23

See Issue sailfishos-patches#23 sailfishos-patches#23

nephros · 2021-11-01T19:39:44Z

Again @b100dian please check my CPP, I'm sure it's horrible.

b100dian · 2021-11-01T22:53:26Z

Again @b100dian please check my CPP, I'm sure it's horrible.

I actually find Qt to be very powerful for not tripping on horrible C++ things and is really nicely used by you:) I'll look in depth ~tomorrow

I need to ramp up on dbus types, but recently I found that as works directly as QStringList and have not found a need for the annotation (nor do I know what a{s} is by just reading https://dbus.freedesktop.org/doc/dbus-specification.html )

nephros · 2021-11-02T10:39:54Z

I need to ramp up on dbus types, but recently I found that as works directly as QStringList and have not found a need for the annotation (nor do I know what a{s} is by just reading https://dbus.freedesktop.org/doc/dbus-specification.html )

I came up with it more or less trial-and-error, and IIRC I got compilation errors when trying 'as'. It's possible something else was the root cause for those though.

src/qml/RestartServicesDialog.qml

b100dian · 2021-11-02T22:16:04Z

src/qml/patchmanager.cpp

    });

+    QDBusPendingCallWatcher *watchGetToggleServicesList = new QDBusPendingCallWatcher(m_interface->getToggleServicesList(), this);
+    connect(watchGetToggleServicesList, &QDBusPendingCallWatcher::finished, [this](QDBusPendingCallWatcher *watcher){


I get no clue from the documentation that this fires any more than once.
I think it is also true that patch updates are notified only when entering the PM settings page and the Restart Services is finicky in the sense that it doesn't show if you first open the menu. I am not 100% sure as I haven't debugged this.

For example, I had to close and re-open the Settings app to get more than one item in the RestartServicesDialog Repeater.

So I guess the call for watcher can be moved to the getter?

Actually this call is useful in the qml page, so it could be made synchronous with no devastating consequences.
The repeated query needs to be done for the boolean that makes the menu entry appear.

Btw, thanks, I did not know what Restart services did until today!

I get no clue from the documentation that this fires any more than once.

Hmm, that's suboptimal ;)

I think it is also true that patch updates are notified only when entering the PM settings page and the Restart Services is finicky in the sense that it doesn't show if you first open the menu. I am not 100% sure as I haven't debugged this.

For example, I had to close and re-open the Settings app to get more than one item in the RestartServicesDialog Repeater.

The easiest way to get it to appear is to unapply and re-apply a patch (or preferrably, more than one from different categories).

So I guess the call for watcher can be moved to the getter?

I'll play around with that and see how it goes. THanks for the thorough testing.

okay, moved to getter in e63cc87 - seems to work.

@b100dian as I didn't completely get your second comment - is that what you had in mind?

Yes, that is exactly what I was suggesting - a plain synchronous getter. I am not sure about "seems to work" - did you also see problems with the one-off triggered callback from the original version (and existing pattern)?

(While I usually do prefer asynchronous calls, this may not be one of the cases to sweat on).

Yes, that is exactly what I was suggesting - a plain synchronous getter. I am not sure about "seems to work" - did you also see problems with the one-off triggered callback from the original version (and existing pattern)?

Not really. One thing I'm not sure about is Signals. Do we need one, when to fire it, when/how it is used form QML when constructing/reading the model etc.

I think that since the model is not changed without QML calling the getter we won't need the toggleServicesListChanged and QML will only call the getter once when the dialog is shown up (or: every time).

So basically if the appsNeedRestart is working correctly for the menu to appear, the getter will too.

(I still don't have an explanation why appsNeedRestart works more than once though. Maybe it so happens that it is only set from false to true only once and from there the getToggleServices is called when you navigate to the page)

nephros · 2021-11-04T13:16:37Z

src/qml/RestartServicesDialog.qml

+            Repeater {
+                model: PatchManager.appsToRestart
+                delegate: Component { TextSwitch {
+                    text: qsTranslate("Sections", modelData)


This works for all languages but English, because English doesn't have a dedicated translation and so uses the (lowercase) original strings.

~~Is that a serious issue from your POV? I.e., serious enough to resurrect and maintain the English TS file?~~

My first thought was: So what, then it just uses lower case strings.
Or maybe this dialog (its textual context) can phrased in a way, that this is fine without having these strings starting with a capital letter?
But as you seem to have looked at it in "real life", just decide how to proceed.

Resolved per #153

Nah, totally minor issue that just came up along the way.

Thanks for fixing this!

Just to document that Transifex uses "translated" strings from the source file as source strings, when the are available (which makes sense IMO).

Side note: Thus this change changed the source strings of all 14 PM-categories for all 14 translations. And because Transifex does not allow programmatic changes at Transifex (e.g., per RegEx), I manually adapted these 14 terms for 13 translations (all, but zh). Maybe scripted downloading, programmatic changing the strings locally and scripted uploading would have been easier, but that would have required learning how to use Transifex's web API, which I wanted to avid this time.

@nephros, please use the exclamation mark only if something went wrong.
Unfortunately the classic conventions for writing dialogues are less obeyed than in the early 2000s, but I still would like to stick to them.

! means: Attention, something went wrong.

... (concluding, i.e., at the end of a phrase or sentence) means: A new window is going to be opened. This is most suitable for choices (which would open a new window).

There are more, which I do not remember spontaneously.

The dialogues in MS-Windows and most of KDE and Gnome are adhering to these conventions, so people are used to them (even most not consciously).

b100dian · 2021-11-08T23:43:31Z

Just re-tested this and it works nicely

according to https://github.com/sailfishos-patches/patchmanager/wiki/Terms-and-wording https://github.com/sailfishos-patches/patchmanager/pull/136/files#r745053026

Olf0

Looking fine.

nephros · 2021-11-10T14:03:02Z

So, would you like this in now, or do we wait until after 3.2?

@Olf0 as you have taken the mantle of release manager for the next couple of versions, please feel free to hit that green button whenever you think its appropriate.

Olf0 · 2021-11-10T18:45:43Z

So, would you like this in now, or do we wait until after 3.2?

@Olf0 as you have taken the mantle of release manager for the next couple of versions,

Huh, have I?
I merely intended to coordinate the releases a bit by asking who wants what in when.
In order to proceed in a structured manner and to avoid multiple, technically or functionally overlapping, major changes in a single release.

please feel free to hit that green button whenever you think its appropriate.

Well, this has become surprisingly big by accumulating a lot of fix-ups, but is clearly disjunct (technically and by functionality) from the principal, major feature for the next release: "32 / 64 bit" per #71 and #109 (comment) (which still has two checkboxes unchecked).

Hence I will happily merge this now.

nephros and others added 2 commits November 1, 2021 20:28

[ui] add display of services to be restarted

c75f45b

See Issue sailfishos-patches#23 sailfishos-patches#23

implement getting list of services to be toggled

5294e23

nephros marked this pull request as draft November 1, 2021 19:39

nephros added 4 commits November 1, 2021 20:47

fixup! [ui] add display of services to be restarted

3a973c3

fixup! [ui] add display of services to be restarted

b95ffeb

Merge branch 'master' into issue-23

c87bf43

fixup! Merge branch 'master' into issue-23

56f6242

nephros added 2 commits November 2, 2021 09:45

Merge remote-tracking branch 'upstream/master' into issue-23

87fc535

cleanup: remove dead code

2aacf83

Merge remote-tracking branch 'upstream/master' into issue-23

f4c017d

b100dian reviewed Nov 2, 2021

View reviewed changes

translate category strings

ce498ad

nephros linked an issue Nov 3, 2021 that may be closed by this pull request

[Usability bug] "Restart preloaded services" enhanced behaviour in PM3 is confusing due to the lack of user feedback #23

Closed

move dbus call to getter

e63cc87

nephros commented Nov 4, 2021

View reviewed changes

update translation source

8a79db3

b100dian approved these changes Nov 4, 2021

View reviewed changes

fixup! translate category strings

d4bf66a

nephros mentioned this pull request Nov 5, 2021

Generate -en.qm file from translation source #185

Merged

nephros added 4 commits November 6, 2021 15:12

Add warning icon and message to apply/unapply notification

3d0c166

update translations

1ddbba8

add missing noop source string for "keyboard"

5a5d238

Merge branch 'master' into pr/issue-23

bf09db9

nephros marked this pull request as ready for review November 9, 2021 08:54

nephros added 2 commits November 10, 2021 09:23

Fix wording

04d24e4

according to https://github.com/sailfishos-patches/patchmanager/wiki/Terms-and-wording https://github.com/sailfishos-patches/patchmanager/pull/136/files#r745053026

Merge remote-tracking branch 'upstream/master' into pr/issue-23

9e67717

Olf0 approved these changes Nov 10, 2021

View reviewed changes

nephros added the UX design and behaviour of UI components label Nov 10, 2021

Olf0 merged commit 348e397 into sailfishos-patches:master Nov 10, 2021

nephros deleted the issue-23 branch January 17, 2023 09:22

Show list of things to be restarted on the "Restart Services" dialog #136

Show list of things to be restarted on the "Restart Services" dialog #136

Uh oh!

Conversation

nephros commented Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nephros commented Nov 1, 2021

Uh oh!

b100dian commented Nov 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nephros commented Nov 2, 2021

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nephros Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nephros Nov 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Olf0 Nov 4, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Olf0 Nov 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Olf0 Nov 8, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

b100dian commented Nov 8, 2021

Uh oh!

Olf0 left a comment

Choose a reason for hiding this comment

Uh oh!

nephros commented Nov 10, 2021

Uh oh!

Olf0 commented Nov 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nephros commented Nov 1, 2021 •

edited

Loading

b100dian commented Nov 1, 2021 •

edited

Loading

nephros Nov 3, 2021 •

edited

Loading

nephros Nov 4, 2021 •

edited

Loading

Olf0 Nov 4, 2021 •

edited

Loading

Olf0 Nov 5, 2021 •

edited

Loading

Olf0 Nov 8, 2021 •

edited

Loading