Drop tab-based UI for the about page
ClosedPublic

Authored by apol on Nov 26 2018, 5:24 PM.

Details

Summary

Integrates better with Kirigami.

Diff Detail

Repository
R134 Discover Software Store
Branch
master
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 5463
Build 5481: arc lint + arc unit
apol created this revision.Nov 26 2018, 5:24 PM
Restricted Application added a project: Plasma. · View Herald TranscriptNov 26 2018, 5:24 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.Nov 26 2018, 5:24 PM
ngraham added a reviewer: VDG.Nov 26 2018, 6:06 PM

Looks much better visually! But something about the bugDisplay looks wrong. Right now it's recommending that people send bugs via email.

Integrating it in one page is definitely an improvement!
Some detailed comments (I am aware that those things introduced with this layout change, they just become apparent now):

  1. "About" at the top of the page and "About Discover..." are redundant. I'd recommend just integrating the Copyright and license lines above the hline and removing the "About Discover..." heading
  2. "Libraries" -> "Libraries used" (might be a bit too long for a tab title but as a title on a page it's perfectly fine)
  3. I'd maybe remove the "Please use..." line above the authors because we have the "Report Bug..." button at the top anyway so it would be redundant. If you fear that users might miss the button up there, I'd opt for moving the button elsewhere rather than having two different places for bug reporting on the same page
leinir requested changes to this revision.Nov 27 2018, 2:22 PM
leinir added a subscriber: leinir.

It does seem that the bugAddress being an email address is a bit odd... The address given by KAboutData::bugAddress can be either an email address, or a URL, though, so it might make sense to try and handle either case (something as simple as checking for :// in the string would probably work), to make the functionality here functionally equivalent to the qwidget dialogue.

At the same time, though, given that Discover's /is/ an email address... would it perhaps make sense to at least add bugs.kde.org as a link? (or, some other way of adding a secondary link for reports... though that does seem to be veering somewhat outside the scope of this particular patch)

This revision now requires changes to proceed.Nov 27 2018, 2:22 PM

Ah yes, somehow i managed to miss the button @colomar mentions, which does the bug reporting linkage already. Removing the link in favour of just keeping that button seems a sensible option :)

apol updated this revision to Diff 46324.Nov 27 2018, 4:22 PM

Address Dan's concern

apol added a comment.Nov 27 2018, 4:38 PM

Looks much better visually! But something about the bugDisplay looks wrong. Right now it's recommending that people send bugs via email.

No it's not, it's recomending to go to bugs.kde.org or wherever the developer pointed at.

Well then Discover is setting the wrong information :)

Can we have an updated screenshot if the last code change resulted in any visual changes?

apol added a comment.Nov 27 2018, 4:43 PM

Well then Discover is setting the wrong information :)

Can we have an updated screenshot if the last code change resulted in any visual changes?

Ah you're right. Let's try putting the button there anyway?

Yeah, I think a "Report a bug!" button would be fine.

apol updated this revision to Diff 46328.Nov 27 2018, 4:49 PM

Remove the bug reporting weirdness

Looks fantastic!

Could we put the bug report button on the toolbar, maybe? It looks a bit odd just sitting in the content view like that.

Also, and I don't know if this is in Discover or the proposed new page, but the item in Discover's sidebar should match the page title and say "About", not "Help."

apol added a comment.Nov 27 2018, 4:56 PM

Looks fantastic!

Could we put the bug report button on the toolbar, maybe? It looks a bit odd just sitting in the content view like that.

It was on the last version. I could easily see people going to send developers an e-mail instead of reporting the issue properly, that's why I put it there.
Did you like it better up there?

Yeah, I did prefer it in the toolbar. I think that actually makes it more prominent, not less. If we are worried about people sending the developers too many emails, we could de-emphasize the email address. The current UI just uses a tiny button which seems adequately obfuscatory :) Or maybe the developer's name could be a mailto link instead of having the address itself visible in plain view?

apol updated this revision to Diff 46390.Nov 28 2018, 9:53 AM

Move report action back to the contextual actions

leinir accepted this revision.Nov 28 2018, 9:55 AM

Lovin' it - i also like how sort of... well, how little code there really is here, QML done absolutely the right way, only the presentation and that's it, nifty :)

This revision is now accepted and ready to land.Nov 28 2018, 9:55 AM

Wouldn't it make sense to provide an AboutPage in Kirigami (KF5::kirigami-whatever-that-can-be-tier-2 :D) like kxmlgui has it, for use in e.g. Itinerary and what not?

apol added a comment.Nov 28 2018, 11:51 AM

Yes, yes, on it.......

apol updated this revision to Diff 46479.Nov 29 2018, 2:57 PM

Adopt the AboutPage upstream in Kirigami

This revision was automatically updated to reflect the committed changes.