Polish Notifier Plasmoid's UI
ClosedPublic

Authored by ngraham on Jul 20 2018, 2:53 AM.

Details

Summary

This patch implements the following changes to Discover's Update Notifier plasmoid:

  • Improve the layout of the controls within the window
  • Update the button text to reflect what it will actually do (the previous text was misleading since it did not in fact perform the updates; it just opened Discover)
  • Update the tooltip text to be clearer
  • Give the button an icon to match its plasmoid icon
Test Plan

Before:

After:

Diff Detail

Repository
R134 Discover Software Store
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
ngraham created this revision.Jul 20 2018, 2:53 AM
Restricted Application added a project: Plasma. · View Herald TranscriptJul 20 2018, 2:53 AM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Jul 20 2018, 2:53 AM
ngraham edited the test plan for this revision. (Show Details)Jul 20 2018, 2:54 AM

+1 for the looks

notifier/plasmoid/contents/ui/Full.qml
27

We typically do that where the component is used, not in its definition

29–30

width: parent.width instead of those nachors
Otherwise the label won't wrap if it's too long (localization)

35–36

I would prefer if you still used a ColumnLayout for those two elements instead of a gazillion anchors

esedgh added a subscriber: esedgh.Jul 20 2018, 9:00 AM

Just a quick question. Why does it have to be so big and waste so much space? It still feels kind of weird.

Thank you Nate, btw. You are doing amazing work everywhere.

ngraham updated this revision to Diff 38136.Jul 20 2018, 1:07 PM
ngraham marked 3 inline comments as done.

Address review comments

Just a quick question. Why does it have to be so big and waste so much space? It still feels kind of weird.

Thank you Nate, btw. You are doing amazing work everywhere.

Thanks for the kind words! Plasmoids have a fixed window size, so it's often tricky to get them to look right, and it's common for the window to feel either too small or too big. But if we added more content, like a scrolling list of the available updates, I'm sure people would complain, "this window is too small! I have so much room on my screen so why does it need to be a tiny scrolling window!?"

It's a tough problem to solve without substantial design changes.

ngraham edited the test plan for this revision. (Show Details)Jul 20 2018, 1:08 PM

Maybe add a second button "Update" that does update right away without viewing the available update list? Most users probably won't be interested in which packages are updated, but just want to stay up to date?

broulik added inline comments.Jul 20 2018, 1:17 PM
notifier/plasmoid/contents/ui/Full.qml
29–30

That's the default anyway

37

anchors.centerIn: parent does both at once :)

40

Keep the horizontalAlignment

I'd thought of that idea too, but it requires more substantial changes than just playing around with the UI, so it'll have to be done in another patch.

apol accepted this revision.Jul 20 2018, 1:40 PM
This revision is now accepted and ready to land.Jul 20 2018, 1:40 PM
This revision was automatically updated to reflect the committed changes.
ngraham marked 3 inline comments as done.Jul 20 2018, 2:01 PM

@broulik addressed your remaining comments in another commit. Sorry for missing them the first time around; flaky bus wifi didn't show me the email for your comments until after I had already landed the patch, following Aleix's approval.

apol added a comment.Jul 21 2018, 1:04 AM

Maybe add a second button "Update" that does update right away without viewing the available update list? Most users probably won't be interested in which packages are updated, but just want to stay up to date?

Then they should use a distribution that upgrades on the background, much like snap does.

In D14238#295426, @apol wrote:

Maybe add a second button "Update" that does update right away without viewing the available update list? Most users probably won't be interested in which packages are updated, but just want to stay up to date?

Then they should use a distribution that upgrades on the background, much like snap does.

They still might want to choose when to update? Like I would not run updates right before some kind of deadline.

apol added a comment.Jul 23 2018, 2:28 PM
In D14238#295426, @apol wrote:

Maybe add a second button "Update" that does update right away without viewing the available update list? Most users probably won't be interested in which packages are updated, but just want to stay up to date?

Then they should use a distribution that upgrades on the background, much like snap does.

They still might want to choose when to update? Like I would not run updates right before some kind of deadline.

I don't think that's something the user should worry about. Again, see snap model.