Bring back embedded top3 reviews on the ApplicationPage
ClosedPublic

Authored by apol on Feb 1 2018, 5:35 PM.

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 1 2018, 5:35 PM
Restricted Application added a project: Plasma. · View Herald TranscriptFeb 1 2018, 5:35 PM
Restricted Application added a subscriber: plasma-devel. · View Herald Transcript
apol requested review of this revision.Feb 1 2018, 5:35 PM
ngraham requested changes to this revision.EditedFeb 1 2018, 9:48 PM
ngraham added a subscriber: ngraham.

Nice! There are a few polish changes I'd like to see:

  • Add a header label saying "Top reviews", since it's otherwise not obvious what users are looking at ("Newest reviews?" "Developer's favorite reviews?"). We could probably replace the blue line with this header unless you deliberately avoided changing any strings to squeak it in for 5.12)
  • Change the label that says "Show Reviews" to "Show more reviews" (unless you deliberately avoided changing any strings to squeak it in for 5.12)
  • Could we increase the vertical padding between each review a bit?
  • For KDenlive's entry, I noticed three extra dummy reviews:

This revision now requires changes to proceed.Feb 1 2018, 9:48 PM
ngraham edited the summary of this revision. (Show Details)
apol updated this revision to Diff 26856.Feb 10 2018, 1:44 AM

Addressed Nate's comments

ngraham added a comment.EditedFeb 10 2018, 2:43 AM

Getting there!

  • Change the text "Show reviews" to "Show more reviews"
  • the "Reviews" header text should maybe be a bit smaller, so as not to visually compete with the app name, as some apps have short descriptions and the two headers appear close together
  • Maybe change the "Reviews" header to "Top Reviews". Judgment call, not necessarily required
  • The increased padding between the reviews is good, but now the divider line is misplaced. It looks really weird not being equidistant between each review. If we can't change that easily, I would advocate getting rid of the line entirely, as the padding is now sufficient to visually separate the reviews.
This revision was not accepted when it landed; it landed in state Needs Review.Feb 12 2018, 6:39 PM
This revision was automatically updated to reflect the committed changes.

[We will iterate over this incrementally since it's on master]