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

Reply via email to