Provide the source selection as a contextual action
ClosedPublic

Authored by apol on Feb 22 2018, 7:37 PM.

Details

Summary

Instead of the dreaded link button.

BUG: 390464

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.
apol created this revision.Feb 22 2018, 7:37 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 22 2018, 7:37 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.Feb 22 2018, 7:37 PM
apol updated this revision to Diff 27816.Feb 22 2018, 7:45 PM

Polish

ngraham requested changes to this revision.Feb 22 2018, 7:46 PM
ngraham added a subscriber: ngraham.

I see what you're going for here, but the "Sources" menu you're trying to add still doesn't actually work, same as the Sort menu. Is there an open Kirigami patch that this depends upon?

Also now there's a textual bug in the Description:

This revision now requires changes to proceed.Feb 22 2018, 7:46 PM

OK, the i18n error is gone now. But do we really need that "also available in XXXX" text at the end of the Description, anyway? Apparently nobody reads it. Maybe we should focus on polishing the main source chooser UI and get rid of that little vestigial one.

apol updated this revision to Diff 27842.Feb 23 2018, 11:39 AM

Remove the description part of origin switching

apol added a comment.Feb 23 2018, 11:42 AM

I see what you're going for here, but the "Sources" menu you're trying to add still doesn't actually work, same as the Sort menu. Is there an open Kirigami patch that this depends upon?

No, please look into how it's possible that it's not working for you. I'll be happy to fix provided the information.
You can play around with kirigami/tests/actionsMenu.qml

Got it working. I think it's better than what we have now, at least. However, the use of checkboxes in the menu is inappropriate: since the entries are mutually exclusive, we should use radio buttons instead (Same with the sort menu; see https://bugs.kde.org/show_bug.cgi?id=391144)

ngraham accepted this revision.Mar 6 2018, 2:45 PM

Turning the checkboxes into radio buttons is tracked with https://bugs.kde.org/show_bug.cgi?id=391144.

One more visual papercut: The menu is not wide enough to accommodate all the text, so long strings get cut off:

This revision is now accepted and ready to land.Mar 6 2018, 2:45 PM
ngraham requested changes to this revision.Mar 6 2018, 2:46 PM
This revision now requires changes to proceed.Mar 6 2018, 2:46 PM
apol added a comment.Mar 6 2018, 3:35 PM

This is an issue in qqc2-desktop-style, not something to solve from Discover.

ngraham accepted this revision.Mar 6 2018, 5:48 PM

OK then!

This revision is now accepted and ready to land.Mar 6 2018, 5:48 PM
cfeck added a subscriber: cfeck.Mar 15 2018, 3:53 AM

Was this committed?

This revision was automatically updated to reflect the committed changes.