D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#389580 <https://phabricator.kde.org/D17975#389580>, @filipf wrote
  
  > I'd much rather if there was some padding added in Breeze and then you only fix the wrong spacing to right of the notification. Even if that wouldn't work, I don't get the sense we've fully explored other solutions yet.
  
  
  There are other problems though besides the right margin of the icon. The heading can't be lifted to stop the icon from towering over it because the theme provides its own padding and, in essence, //locks it into place//.
  
  In other words, the padding here **varies depending on the amount of notification content**, which I don't think is something a theme can account for.
  
  Modifying Breeze to add padding would restrict its ability to manipulate the notification content fully.
  
  In D17975#389584 <https://phabricator.kde.org/D17975#389584>, @ngraham wrote:
  
  > So what's going on here? Other themes add more padding? If so, I don't see how that's something we can possibly account for, and that shouldn't affect our ability to improve our own stuff.
  
  
  We may be able to account for it, but our hands are tied here I'm afraid because of the technical difficulties of making the Breeze theme doing what I want the notifications to do in this patch (vary padding by notification content).

INLINE COMMENTS

> davidedmundson wrote in NotificationItem.qml:33
> Much better.
> 
> Only part I don't understand, then ship it! from me.
> 
> Assuming there's no icon
> 
> For multiline
> 
> - 1 small space above 1 small space under.
> 
> For one line we effectively get:
> 
> - 0 margin on top, 0.5 under
> 
> Why the 0.5? Surely that would be inconsistent?
> 
> (In all your examples the icon is bigger so we don't see it)

You're right! I never tested it with just the heading, There's an extra 0.5 * units.smallSpacing of padding there. But I needed that 0.5 units to provide even padding to notifications that are "Heading + One line of description".

Is there any way I can put in a condition that if it's just a heading that it not put that 0.5 * units.smallSpacing there?

REPOSITORY
  R120 Plasma Workspace

REVISION DETAIL
  https://phabricator.kde.org/D17975

To: rooty, #vdg, #plasma, ngraham
Cc: Codezela, abetts, filipf, davidedmundson, hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20190108/cdedede5/attachment.html>