Expose more URLs relevant to the app
ClosedPublic

Authored by ngraham on Jan 26 2018, 11:23 PM.

Details

Summary

Make the help, bug tracker, and donation URLs available to the backends, and display them on the App page.

Currently working on a better design to make the metadata section not look so intimidating for apps that behave well and expose all these URLs--but that can go in later, since this has to land on master.

Test Plan

Tested in KDE Neon with apps available from multiple backends. Here's an example of how it looks with Gedit, which defines all the URLs:

Diff Detail

Repository
R134 Discover Software Store
Branch
more-URLs
Lint
No Linters Available
Unit
No Unit Test Coverage
ngraham created this revision.Jan 26 2018, 11:23 PM
Restricted Application added a project: Plasma. · View Herald TranscriptJan 26 2018, 11:23 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
ngraham requested review of this revision.Jan 26 2018, 11:23 PM
ngraham edited the test plan for this revision. (Show Details)Jan 26 2018, 11:24 PM

(for master, since it includes a string change)

apol added inline comments.Jan 26 2018, 11:28 PM
libdiscover/resources/AbstractResource.h
134

Maybe they could return an empty url by default? This way they don't have to be implemented right away.

apol added inline comments.Jan 26 2018, 11:29 PM
discover/qml/ApplicationPage.qml
337

No need to put it in a separte property. And specially don't add the exact same 2 properties!

345

it's missing the rest, no?

ngraham updated this revision to Diff 26024.Jan 27 2018, 12:29 AM
  • Don't create unnecessary stub functions
  • Don't unnecessarily re-define variables ApplicationPage.qml
  • Show all the new URLs by default (when available)
ngraham marked 3 inline comments as done.Jan 27 2018, 12:30 AM
ngraham edited the test plan for this revision. (Show Details)
apol added inline comments.Jan 27 2018, 12:36 AM
discover/qml/ApplicationPage.qml
39

Here actually we probably want to be a bit smarter. If it's updateable we probably want to reflect it as well.

351

How about:

QQC2.Label {
    visible: donationButton.visible
    Layout.alignment: Qt.AlignRight
    text: i18n("Make a Donation:")
}
LinkButton {
    id: donationButton
    visible: text.length > 0
    text: application.donationURL
    onClicked: Qt.openUrlExternally(donationURL)
    elide: Text.ElideRight
    Layout.fillWidth: true
    horizontalAlignment: Text.AlignLeft
}

This way we don't need to have random top-level properties for every single thing.
Also note that referring to properties by name is considered expensive. So it should have been appInfo.donationURL).

ngraham updated this revision to Diff 26032.Jan 27 2018, 10:41 AM

Be smarter about how we find and display the links

ngraham marked 2 inline comments as done.Jan 27 2018, 10:42 AM
ngraham updated this revision to Diff 26033.Jan 27 2018, 10:43 AM

Fix unintentional whitespace change

ngraham edited the summary of this revision. (Show Details)Jan 27 2018, 10:44 AM
apol accepted this revision.Jan 27 2018, 2:45 PM

Other than that, LGTM.

discover/qml/ApplicationPage.qml
348

Donation:? Donate:?

"Make a donation" reads a bit too long to me for such a title.

This revision is now accepted and ready to land.Jan 27 2018, 2:45 PM
ngraham added inline comments.Jan 27 2018, 2:54 PM
discover/qml/ApplicationPage.qml
348

"Donate: sounds good. "Make a Donation:" would probably be 4 lines long in German, anyway. :)

ngraham updated this revision to Diff 26062.Jan 27 2018, 2:55 PM

"Make a Donation" -> "Donate"

ngraham marked 2 inline comments as done.Jan 27 2018, 2:55 PM
ngraham closed this revision.Jan 27 2018, 3:49 PM