D19874: [Kickoff] Reduce the margins of KickoffItem, KickoffHighlight and use smallSpacing

2019-04-05 Thread Noah Davis
This revision was automatically updated to reflect the committed changes. Closed by commit R119:51f7912368e6: [Kickoff] Reduce the margins of KickoffItem, KickoffHighlight and use… (authored by ndavis). REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D198

D19874: [Kickoff] Reduce the margins of KickoffItem, KickoffHighlight and use smallSpacing

2019-04-05 Thread Nathaniel Graham
ngraham accepted this revision. ngraham added a comment. This revision is now accepted and ready to land. I agree that this makes it feel more left-heavy. However I still think this patch is an improvement because the artificial margins we're getting rid of don't really solve that problem eit

D19874: [Kickoff] Reduce the margins of KickoffItem, KickoffHighlight and use smallSpacing

2019-04-05 Thread Noah Davis
ndavis added a comment. In D19874#444153 , @filipf wrote: > This makes Kickoff even a bit more more left-centered = looking like it's wasting horizontal space. That's why I prefer the way it was before, but I tested the patch and everything seem

D19874: [Kickoff] Reduce the margins of KickoffItem, KickoffHighlight and use smallSpacing

2019-04-05 Thread Filip Fila
filipf added a comment. This makes Kickoff even a bit more more left-centered = looking like it's wasting horizontal space. That's why I prefer the way it was before, but I tested the patch and everything seems symmetric at least. It seems that in the screenshots the "Small" font is set

D19874: [Kickoff] Reduce the margins of KickoffItem, KickoffHighlight and use smallSpacing

2019-04-05 Thread Noah Davis
ndavis added a comment. I haven't tested this with many different fonts, but the current version works well for Noto Sans at 10, 11 and 12 pts. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19874 To: ndavis, #plasma, #vdg, ngraham, hein Cc: hein, ngraham, f

D19874: [Kickoff] Reduce the margins of KickoffItem, KickoffHighlight and use smallSpacing

2019-04-05 Thread Noah Davis
ndavis updated this revision to Diff 55502. ndavis marked 2 inline comments as done. ndavis added a comment. - Remove commented out bit of old code REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19874?vs=55500&id=55502 BRANCH KickoffItem-margins (

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-04-05 Thread Noah Davis
ndavis updated this revision to Diff 55500. ndavis added a comment. Use smallSpacing for KickoffItem and KickoffHighlight REPOSITORY R119 Plasma Desktop CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19874?vs=54292&id=55500 BRANCH KickoffItem-margins (branched from master) REV

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-04-05 Thread Nathaniel Graham
ngraham added a comment. Thanks @hein, now we just need the requested code changes. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19874 To: ndavis, #plasma, #vdg, ngraham, hein Cc: hein, ngraham, filipf, rooty, plasma-devel, jraleigh, GB_2, ragreen, Pitel,

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-04-05 Thread Eike Hein
hein accepted this revision. hein added a comment. In D19874#443917 , @filipf wrote: > @hein what's your opinion on reducing the margins? LGTM. REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19874 To: nd

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-04-05 Thread Filip Fila
filipf added a subscriber: hein. filipf added a comment. @hein what's your opinion on reducing the margin? REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19874 To: ndavis, #plasma, #vdg, ngraham Cc: hein, ngraham, filipf, rooty, plasma-devel, jraleigh, GB_2,

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-04-05 Thread Nathaniel Graham
ngraham added a comment. Ping! REPOSITORY R119 Plasma Desktop REVISION DETAIL https://phabricator.kde.org/D19874 To: ndavis, #plasma, #vdg, ngraham Cc: ngraham, filipf, rooty, plasma-devel, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas,

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-03-19 Thread Noah Davis
ndavis added a comment. Another thing is the ratio between the left/right margins and the top/bottom margins varies with font size, so maybe Kickoff really does need some fixing up. The height is `2 * units.smallSpacing`, plus the maximum value of the icon, title+subtile or arrow height.

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-03-19 Thread Noah Davis
ndavis added a comment. In D19874#434319 , @ngraham wrote: > +1 conceptually. Since Kickoff has its own side margins, these additional margins in the items themselves are just unnecessary. I tried to see how they would look with no margin

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-03-19 Thread Krešimir Čohar
rooty added a comment. You should test this with all four orientations. And if you redo the screenshots do them like this: Category - before after Long description - before after It's really hard to appreciate the differences this way. REPOSITORY R119 Plasma Desktop REVISI

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-03-19 Thread Nathaniel Graham
ngraham requested changes to this revision. ngraham added a comment. This revision now requires changes to proceed. +1 conceptually. Since Kickoff has its own side margins, these additional margins in the items themselves are just unnecessary. I think there's already enough indentation with c

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-03-19 Thread Filip Fila
filipf added a comment. I actually like the right margin because it contributes to visual hierarchy when headings are present: F6701391: image.png As for long descriptions, how often would you say it occurs that you have one? From my experience

D19874: [Kickoff] Reduce the margins of KickoffItem

2019-03-19 Thread Krešimir Čohar
rooty added a comment. Looks a lot better, +1. Please check whether there is subpixel placement of either the fonts or the selection rectangle with some other fonts (because gridUnit is being multiplied by a non-integer). I know it doesn't make sense but @flipwise and I have had to modi