Display version alongside source, and use a combobox to switch between them
AbandonedPublic
Actions

Authored by ngraham on Jan 19 2018, 3:38 AM.

Details

Reviewers

apol
colomar
abetts
andreaska

Group Reviewers

Discover Software Store
VDG

Summary

Joined the version number and source together to communicate their relationship when more then one source is present
Implemented a combobox to switch versions instead of blue-colored link text and an overlay

This patch includes a string change, so it cannot go into 5.12.

Errata:

This new UI makes https://bugs.kde.org/show_bug.cgi?id=389186 more visible (though it's already present, just less visible with the current UI)
Ubuntu provides long and ugly version strings most of the time, which sometimes makes the combobox overflow the window when it's narrow--and typical width-limiting/eliding measures don't seem to work with QQC comboboxes

Test Plan

Tested in KDE Neon.

Krita, before:

Krita, after:

Endless Sky, before:

Endless Sky, after:

Blender, before:

Blender, after:

Mobile UI, before:

Mobile UI, after:

Version chooser UI, before:

Version chooser UI, after:

Diff Detail

Repository

R134 Discover Software Store

Branch

arcpatch-D9976

Lint

No Linters Available

Unit

No Unit Test Coverage

ngraham requested review of this revision.Jan 19 2018, 3:38 AM

ngraham created this revision.

ngraham edited the summary of this revision. (Show Details)Jan 19 2018, 3:43 AM

ngraham mentioned this in D9974: Improve header and metadata display.Jan 19 2018, 3:58 AM

ngraham edited the summary of this revision. (Show Details)Jan 19 2018, 5:10 AM

ngraham added a subscriber: romangg.

romangg added inline comments.Jan 19 2018, 5:18 AM

libdiscover/resources/AbstractResource.cpp
224	Can you keep it const when adding const qualifier to `state()`?

ngraham added inline comments.Jan 19 2018, 5:22 AM

libdiscover/resources/AbstractResource.cpp
224	I'm afraid I'm not sure what you mean, being a novice at this. Can you ELI5?

romangg added inline comments.Jan 19 2018, 5:29 AM

libdiscover/resources/AbstractResource.cpp
224	Sure. The `const` qualifier specifies that this function does not change any member variables in `AbstractResource`. If you still try to change one in the function, you get a compile error. This happens also if you call some other non-const member function inside the const function. You removed the qualifier probably because you now call `state()` below and it wouldn't compile otherwise. But the `state()` function from its very name could be made const probably just as well (maybe this should be put in a separate patch, this one here then depends on). And then `displayOrigin()` can stay const.

ngraham added inline comments.Jan 19 2018, 2:56 PM

libdiscover/resources/AbstractResource.cpp
224	Thanks for the very clear explanation; now I understand. Looks like `state()` is a virtual function that gets overridden by child classes? I'll see if I can figure out what to do here.

Make state() into const functions

Restricted Application added a project: Plasma. · View Herald TranscriptJan 19 2018, 3:07 PM

Restricted Application added a subscriber: plasma-devel. · View Herald Transcript

ngraham marked 4 inline comments as done.Jan 19 2018, 3:08 PM

ngraham edited reviewers, added: andreaska; removed: andreask.Jan 19 2018, 4:14 PM

januz awarded a token.Jan 19 2018, 5:46 PM

colomar added a comment.Jan 19 2018, 7:15 PM

This comment was removed by colomar.

Design look good

This revision is now accepted and ready to land.Jan 19 2018, 7:20 PM

I think that when at the bottom, having the combobox is okay because it's not too prominent. The only problem I see is that it means that changing a combobox changes the content above it, which is not usually expected behavior for a combobox.

ngraham mentioned this in R134:93bd781900b5: Improve header and metadata display.Jan 20 2018, 2:25 AM

Merge master and fix the issue of "installed:" getting appended to the origin string

ngraham edited the summary of this revision. (Show Details)Jan 20 2018, 8:19 PM

Merge branch 'master' into arcpatch-D9976

I think it's premature the adoption of the combo box, adding the version could make sense.

libdiscover/backends/PackageKitBackend/PackageKitResource.cpp
126	Seems like we are hitting a bug in PackageKit-Qt here, this is not right.

This revision now requires changes to proceed.Jan 24 2018, 3:29 PM

ngraham edited the summary of this revision. (Show Details)Jan 24 2018, 3:31 PM

ngraham edited the test plan for this revision. (Show Details)

ngraham added inline comments.

libdiscover/backends/PackageKitBackend/PackageKitResource.cpp
126	I filed a bug on them and they said they expect us to process the string.

ngraham mentioned this in D10091: Combine source and version into a single piece of metadata.Jan 25 2018, 3:14 AM

Source-and-version-unification patch: D10091

Will produce a ComboBox patch if and when that's accepted, as it doesn't make as much sense without first unifying the source and version.

Revision Contents
Changeset List

			Path	Packages
M			discover/qml/ApplicationPage.qml (91 lines)
M			libdiscover/backends/FlatpakBackend/FlatpakResource.h (2 lines)
M			libdiscover/backends/FlatpakBackend/FlatpakResource.cpp (2 lines)
M			libdiscover/backends/KNSBackend/KNSResource.h (2 lines)
M			libdiscover/backends/KNSBackend/KNSResource.cpp (2 lines)
M			libdiscover/backends/PackageKitBackend/LocalFilePKResource.h (2 lines)
M			libdiscover/backends/PackageKitBackend/PackageKitResource.h (2 lines)
M			libdiscover/backends/PackageKitBackend/PackageKitResource.cpp (6 lines)
M			libdiscover/backends/SnapBackend/SnapResource.h (2 lines)
M			libdiscover/backends/SnapBackend/SnapResource.cpp (2 lines)
M			libdiscover/resources/AbstractResource.h (2 lines)
M			libdiscover/resources/AbstractResource.cpp (3 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	25621	fe508ff		Jan 19 2018, 3:38 AM	★	★
Diff 2	25640	12576d2	Make state() into const functions	Jan 19 2018, 3:07 PM	★	★
Diff 3	25692	768be3c	Merge master and fix the issue of "installed:" getting appended to the origin…	Jan 20 2018, 8:18 PM	★	★
Diff 4	25887	ea5325d	- Merge branch 'master' into arcpatch-D9976	Jan 24 2018, 3:24 PM	★	★

Commit	Tree	Parents	Author	Summary	Date
85434bc150e3	2b46f42a4970	4cfab7b8ea20 ea5325ddc537	Nathaniel Graham	Merge branch 'master' into arcpatch-D9976	Jan 24 2018, 2:46 PM
4cfab7b8ea20	224b83c03289	768be3cdc965	Nathaniel Graham	Display version alongside source, and use a combobox to switch between them (Show More…)	Jan 24 2018, 2:43 PM

	Status	Author	Revision
	Abandoned	ngraham	D9976 Display version alongside source, and use a combobox to switch between them
	Closed	ngraham	D9974 Improve header and metadata display