D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Root]

rooty added a comment.


  In D16031#392659 , @ngraham wrote:
  
  > After much discussion in the #VDG  
room, we decided to put this on hold and revisit it for 5.16. The problem here 
is that we're worried that this change won't be enough to stop people from 
wanting to redesign it yet again, and we don't want to produce change fatigue 
here. We're going to focus on producing a timeless design that's good enough 
that nobody wants to redesign it anymore, which is ultimately the best sign 
that you have the design right.
  
  
  agreed
  
  by the way, we might get an idea by/from revisiting old plasma themes too
  
  F6548022: image.png 

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: alexde, raddison, romangg, davidedmundson, rizzitello, abetts, pstefan, 
broulik, rikmills, filipf, rooty, plasma-devel, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D18206: Improve the Look and Feel KCM

[Root]

rooty added a comment.


  +1 from me
  if you want you could drop the article by the way and make it "Choose Look 
and Feel theme"
  it's like "Configure Network Settings" vs "Configure the Network Settings"

REPOSITORY
  R119 Plasma Desktop

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

To: GB_2, #plasma, #vdg
Cc: rooty, rizzitello, abetts, filipf, #vdg, plasma-devel, #plasma, kvanton, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#391560 , @ngraham wrote:
  
  > I believe you'll need to file a sysadmin ticket to get that changed.
  
  
  Will do! Thanks!

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#391371 , @davidedmundson 
wrote:
  
  > @rooty if you want to re-apply for a commit account, I'll approve it.
  
  
  Thank you you guys! One problem though - I used an alias (Pete Cho) on 
identity.kde.org, but i'd like to use my real name, is that possible?

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 49230.
rooty added a comment.


  Use units.smallSpacing for padding consistently except when 
bodyText.lineCount > 1, spruce things up

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=49209=49230

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  You were right! Thank you so much, it was so much easier when I started 
thinking about it as a math problem rather than a design problem. I'm going to 
upload another diff, I hope this one stands up to scrutiny.
  
  I apologize for the very long "implicitHeight" but it had to cover six 
different configurations:
  
  - no icon + heading only
  - no icon + heading + one line of body text
  - no icon + heading + many lines of body text
  - icon + heading only
  - icon + heading + one line of body text
  - icon + heading + many lines of body text.

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 49209.
rooty added a comment.


  Fix lower border cutoff for fonts > 10 pt

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=49208=49209

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  
  
  >> How do we get the icon size to increase in proportion to increasing font 
size
  > 
  > See units.cpp in plasma-framework. It's complex. I don't think that's a 
relevant topic you need to go down
  
  Yeah, also I'm not sure it's the right approach - icons might turn out to be 
HUGE...
  
  In D17975#391025 , @filipf wrote:
  
  > Last diff is much better in general, but Noto Sans 11pt seems to cause 
problems
  >
  > F6539442: image.png 
  
  
  I have an idea how to fix that.

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 49208.
rooty added a comment.


  Correct padding with just icon + heading + no text, make icon padding even 
for 10 pt sized fonts other than Noto Sans

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=49132=49208

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#391008 , @filipf wrote:
  
  > ^ Yes, it's important to note that additional (and unwanted) padding 
underneath the icon can happen with master, while this patch can add the same 
above the icon. From my layman's understanding, there is nothing solid and 
absolute values are based on.
  
  
  There are effectively two problems here:
  
  - How do we get even padding around the **icon itself** without having to 
resort to manipulating the rest of the notification?
  - How do we get the icon size to increase in proportion to increasing font 
size (and/or scaling factor) and, once bigger/smaller, how do we get it to stay 
that very same size regardless of how many lines of text there are?

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#390875 , @ngraham wrote:
  
  > With this patch: F6539252: With patch.png 

  
  
  TLDR: Yup, but that's only **the tip of the iceberg** (of the problems that 
plague //both master and this patch//)
  
  It's the problem I described two comments ago:
  
  > However, if another font is used, the amount of padding below the icon 
changes (Cabin - 5 px, Lato - 3 px etc.). It's because I've been using 
bottomPart or the implicitHeight to make the notification "taller" so as to 
give the icon padding.
  >  Is there a way to give the icon itself padding? I've been using anchors 
but there's nothing I can think of to anchor it to? If I anchor it to the 
notification text in any way, I can add a margin, sure, but it gets //stretched 
out beyond belief// if there's too much text.
  
  I've been using fonts to illustrate the problem but your screenshots //do a 
better job.//
  In addition, changing the height of the **icon** results in several binding 
loops (gross), or in an icon that's not a square (album art), or the situation 
in your second screenshot. Hardly ideal.
  
  I just don't know how to add bottom padding //to the notification icon 
itself// and have the height of the notification adapt accordingly.
  
  P.S. This did, however, shed some light on things that have been bothering me 
about the current master.
  
  > Status quo: F6539251: Without the patch.png 

  
  Should this happen? There's no description yet there's an empty space. The 
icon doesn't resize accordingly. This is what happens in master with fonts 
>10pt and scaling that's a non-integer that's not a multiple of 0.5 (1.1, 1.2, 
1.3 etc.), resulting in padding underneath the icon (not the notification 
text!) and none above. For example:
  F6539300: image.png 
  The plot thickens - another screenshot of **master** (not this patch)
  F6539302: image.png 
  I'm assuming this second one is because of the close button. This doesn't 
happen with this patch but I'm assuming I merely obscured the problem rather 
than solved it.
  
  I'm at a loss as to how to proceed. The padding problems result from:
  
  - in master, because the icon is defined as "units.iconSizes.largeSpacing" 
without any regard for the user's font (or scaling factor actually, except when 
it's 1.5, 2, 2.5, 3)
  - in this patch, because of the units.iconSizes.largeSpacing thing and my 
lack of proficiency in QML and inability to give the icon padding without 
manipulating either its height or that of the entire notification (doing the 
former results in **several binding loops** OR an icon that's **distorted in 
size** and isn't a square... doing the latter results in the problem depicted 
in your screenshots)

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#390336 , @rooty wrote:
  
  > Is there a way to give the icon itself padding? I've been using anchors but 
there's nothing I can think of to anchor it to? If I anchor it to the 
notification text in any way, I can add a margin, sure, but it gets //stretched 
out beyond belief// if there's too much text.
  
  
  AFter playing around with the code (I'm still very new to this), I came up 
with this:
  
PlasmaCore.IconItem {
id: appIconItem

width: (bodyText.lineCount > 1) ? units.iconSizes.large : 
(notificationItem.height - 2 * units.smallSpacing)
height: (bodyText.lineCount > 1) ? units.iconSizes.large : 
(notificationItem.height - 2 * units.smallSpacing) 
  
  And a dozen binding loops later (wow!) the padding is indeed even with one 
heading + one line of description, regardless of what font is used (and the 
icon is still a square). And it looks fine with 1x, 1.5x, 2x, 2.5x scaling set 
up, but when it's 1.1, 1.2 etc. or the font is >10 pt in size, the icon is 
larger when the lineCount is 1 than when the line count is greater than 1.
  
  This solution seems very strange/crude to me though, and it's not without its 
downsides.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  There is one small problem, with almost every version I've submitted so far.
  The padding above the icon and below the icon is the same (7 px with 1x 
scaling) if Noto Sans is used as the default font, which is basically perfect.
  
  However, if another font is used, the amount of padding below the icon 
changes (Cabin - 5 px, Lato - 3 px etc.).
  It's because I've been using bottomPart or the implicitHeight to make the 
notification "taller" so as to give the icon padding.
  
  Is there a way to give the icon itself padding? I've been using anchors but 
there's nothing I can think of to anchor it to? If I anchor it to the 
notification text in any way, I can add a margin, sure, but it gets //stretched 
out beyond belief//.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 49132.
rooty added a comment.


  Remove unnecessary comments

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=49131=49132

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 49131.
rooty added a comment.


  Use margin in bottomPart instead

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=49110=49131

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D18147: Change certain integers to decimals for even padding; fix uneven padding with heading only notifications

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  Hey @davidedmundson I know I'm a little late but should I use your suggestion 
instead? I just figured out how to do it using Layout.bottomMargin :D

REPOSITORY
  R120 Plasma Workspace

BRANCH
  master

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

To: rooty, #vdg, #plasma, ngraham, davidedmundson
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 49110.
rooty added a comment.


  Change certain integers to decimals for more even padding; fix padding in 
heading-only notifications

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=48943=49110

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  @broulik If the consensus is that the notifications should stay "tight" (no 
padding), I'd very much prefer to add padding to the left and right of the icon 
(it's really stuck-on right now), and to push the notification icon down a bit 
if there's a bodyText.lineCount > 1 to prevent the icon from towering over the 
heading.
  
  F6537400: image.png 

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#389760 , @filipf wrote:
  
  > FWIW if I backport the spacings and margins prior to D3560 
 it works great for me. The symmetry around 
the icon is better than with this diff. (only NotificationItem.qml is modified)
  >
  > This diff:
  >  F6537366: Screenshot_20190109_000448.png 

  >
  > Old:
  >  F6537364: image.png 
  
  
  Try rolling back NotificationPopup.qml too. They should look very similar to 
the ones in this patch.

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#389695 , @broulik wrote:
  
  > We explicitly removed the padding in Plasma 5.9 to reduce popup sizes, this 
effectively reverts this decision…
  
  
  Sort of. This is basically what they looked like:
  F6537353: image.png 
  And the result of D3560  were a slew of 
custom themes that added the padding back. The main justification was:
  
  > Our notification popups are huge compared to other platforms
  
  and that's just not the case. At least not anymore:
  
  F6537359: image.png 
  F6537361: image.png 
  
  I grant that notifications without padding do save room. But it's really not 
that much room we're saving? Especially considering that a notification is 
something a user should be paying attention to.
  As far as text on the buttons is concerned, I'm confident we can make it work 
and that the labels need not be too short or too long, or in other words, that 
this is an issue completely separate from the padding issue.

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: broulik, Codezela, abetts, filipf, davidedmundson, hein, ndavis, 
plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#389613 , @davidedmundson 
wrote:
  
  > BTW, if you want a neater solution: (though consider this optional)
  >  Instead, use Layout.topMargin on the titleBar and Layout.bottomMargin on 
the lowerPart.
  
  
  I love it. It's just that the icon doesn't seem to get the message :/ That's 
why I've been manipulating the height this entire time. I'd like to implement 
this but I don't know how to add padding to IconItem... (I can't think of 
anything to anchor it to - anchoring it to parent.bottom and adding a margin 
stretches the icon out / distorts it)
  
  > There's a logic error. I'll rewrite it so you can see it.
  
  I'm sorry I still don't get it 
  If greater than 1 then return 0.5, otherwise (it's less than or equal to 1 = 
either 0 or 1?) if 0 then 0, otherwise (not zero but 1) return 2.

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

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  > I wouldn't want the attitude of "breaking other themes is unfortunate, 
but.." becoming a pattern because openness to customization is our main selling 
point.
  
  You've made your point. But allow me to flesh it out  - it seems we've 
reached an impasse; in simple terms, you're either going to maintain the themes 
and restrict Plasma's growth or you're going to remedy the problems this widget 
has exhibited and step on a few toes.
  
  You can, in fact, modify Breeze to add padding, sure. But if you do so, you 
can't move the notification text up (or around, if you should want to). I'm 
kind of belaboring the point here - that means you can't get padding that 
stretches and contracts naturally depending on how much notification content 
there is. (I never wanted **just** padding.)
  
  You can also manipulate the notification icon to achieve the same effect that 
I achieved by lowering the value of topMargin of the notification heading. 
However even then, padding that's arrived at through theme manipulation is 
still always going to remain the same.

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

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  @davidedmundson I've tried using
  
implicitHeight: Math.max(appIconItem.valid || imageItem.nativeWidth > 0 ? 
units.iconSizes.large : 0, (mainLayout.height + ((bodyText.lineCount > 1) ? 0.5 
: ((bodyText.lineCount == 0) ? 0 : 2) * units.smallSpacing))) // Add 
units.smallSpacing to correct for the topMargin of mainLayout according to 
number of lines
  
  (I know, it looks ugly...) but it doesn't seem to do anything

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

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In 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 , @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

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#389567 , @Codezela wrote:
  
  > why every notification width is too wide
  >  why its not resize depends on the content on it with some little padding
  
  
  what do you mean? the width of the entire notification?

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

D17975: [Notifications] Add padding to notifications

[Root]

rooty added a comment.


  > Wouldn't it be better to somehow add padding globally?
  
  Not really feasible because you don't necessarily want padding everywhere 
(for example, Adapta does this and i think it looks unsightly)
  F6536644: adapta-popup.png 
  Adding padding **//everywhere//** might involve modifying every heading and 
every icon, that's just not what we want to do (you don't, for instance, want 
padding around system settings sidebar icons which might be a side effect of 
what you're suggesting)
  
  If you limit yourself to only text/headings, then adding padding may make 
your GUI elements look disproportionate (the text gets padding, the icon 
doesn't and gets pushed up and to the left (provided the theme doesn't provide 
you with padding), which is what happened in some of my mockups)
  
  Breaking other desktop themes is unfortunate, but our primary responsibility 
is in my opinion to Breeze - maybe we can find a way to modify Breeze to add 
more padding/cushioning, but then two problems arise:
  
  (1) This would prevent the text from being shifted upward with a line count 
greater than 1 and cause **//even greater inconsistency//** in the margins 
(they would become very asymmetrical) - you could work around this but these 
workaround are probably going to be very //inelegant//
  (1) These modifications would have to be implemented in Breeze Dark, Breeze 
LIght and possibly even Breath (the main theme Manjaro Linux uses), and any 
other theme that wants to follow suit
  (3) This would also open up a couple of other very interesting questions - 
should other Plasma elements be modified, or should the widgets that call for 
them be modified? Should we rely on desktop themes to add padding to "Audio 
Volume" or "Status & Notifications"?

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

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#389443 , @ngraham wrote:
  
  > This is no longer WIP, right?
  
  
  Oh so I can rename it? Sure!

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: abetts, filipf, davidedmundson, hein, ndavis, plasma-devel, kvanton, 
jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D18078: [plasma-workspace-wallpapers] Improve wallpaper selection

[Root]

rooty added a comment.


  wow scrolling down took a while haha
  sure fine with me, crop em (except for Lake Bled?)

REPOSITORY
  R131 Plasma Wallpapers

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

To: filipf, #plasma, #vdg, ngraham
Cc: mart, rooty, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol

D18019: [Digital Clock] Add ability to set a custom date format string

[Root]

rooty added a comment.


  a thousand times yes... this is amazing

REPOSITORY
  R120 Plasma Workspace

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

To: Zren, #plasma, #vdg
Cc: rooty, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#388788 , @ngraham wrote:
  
  > Could you update the images in the Summary section instead of posting new 
ones in comments? The visual history of the patch is not really important to 
preserve, and having the images on top always be current makes reviewing it 
much easier. Thanks!
  
  
  haha sure thing, can i just do the before and after then? no more stream of 
consciousness?

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: davidedmundson, hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  I removed "0.75 * units.smallSpacing" because it's not an integer - the 
result is that the margins are slightly uneven, but considering they weren't 
ever entirely even, and the difference between the margins gets more pronounced 
with increasing font size (and scaling factor), but isn't noticeable, maybe 
it's a nonissue?
  
  That being said, I've also added a line that raises the notification contents 
by units.smallSpacing if there is more than one line of text, to make the 
margins more even. 
  Before: 
  F6533113: before.png 
  After: 
  F6533115: after.png 

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: davidedmundson, hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 48943.
rooty added a comment.


  Use integers/halves and line counts

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=48711=48943

BRANCH
  master

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham
Cc: davidedmundson, hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  Hey guys, I feel silly asking this but I'm using a different computer right 
now - how do I get this computer's arcanist to lock onto and update this diff 
(instead of creating an entirely new diff...)?

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: davidedmundson, hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D18087: Use integers or halves, and correct for text margins using line count

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  notification-padding (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18078: [plasma-workspace-wallpapers] Improve wallpaper selection

[Root]

rooty added a comment.


  Still not sure if we should have also included the grassland
  
  F6507127: jean-pierre-brungs-103087-unsplash.jpg 


REPOSITORY
  R131 Plasma Wallpapers

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

To: filipf, #plasma, #vdg, ngraham
Cc: rooty, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18048: [Connections] Add search ellipsis to network connection search

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch adds an ellipsis to the search function in Connections / System 
Settings.

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  connections-searchellipsis (branched from master)

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

AFFECTED FILES
  kcm/qml/main.qml

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18047: [plasma-desktop] Add search ellipsis to Font Management and Date and Time

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch adds an ellipsis to the filter and search labels in Font 
Management and Date and Time.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fontmn-date-searchellipsis (branched from master)

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

AFFECTED FILES
  kcms/dateandtime/dateandtime.ui
  kcms/kfontinst/kcmfontinst/FontFilter.cpp

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D18044: [kde-cli-tools] Add search ellipsis to label, make label more consistent with HIG

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch renames the label "Find file type or filename pattern" to "Search 
for file type or filename pattern..."

REPOSITORY
  R126 KDE CLI Utilities

BRANCH
  kdeclitools-searchellipsis (branched from master)

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

AFFECTED FILES
  keditfiletype/filetypesview.cpp

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17906: [kirigami] Do not use light font styles for headings (2/3)

[Root]

rooty added a comment.


  In D17906#388052 , @mart wrote:
  
  > can we still keep as light the first level of heading, which is huge (and 
use that sparsingly) and go to levels from 2 onwards where needed smaller and 
bigger?
  
  
  nope, very much against this idea... it //might// work if we used segoe ui, 
but that's about the only instance in which it would

REPOSITORY
  R169 Kirigami

BRANCH
  no-light-headings (branched from master)

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

To: filipf, ngraham, #plasma, #vdg, #frameworks
Cc: rooty, #frameworks, ngraham, #plasma, #vdg, abetts, mart, plasma-devel, 
dkardarakos, apol, davidedmundson, hein

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  @davidedmundson Is there anything I can put in the code to get the icons to 
scale properly? this is a screenshot of master (not this patch):
  
  F6529598: image.png 
  
  I've tried multiplying units.iconSizes.large by the scaling factor, and that 
works, but that's as ballpark a solution as they come...

INLINE COMMENTS

> davidedmundson wrote in NotificationItem.qml:175
> > Is there any way around this? Because if it's not 0.75 then it's shifted 
> > too far down (uneven padding)
> 
> If that's the case, adjust the other side, even adjust the window if you need 
> to.

Nate's right though, I should abandon the idea of totally even padding because 
it'll never hold up as you change fonts or scaling... then again it doesn't 
have to be perfect, right? Just close enough?

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: davidedmundson, hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, 
ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty marked an inline comment as done.
rooty added a comment.


  In D17975#386685 , @ngraham wrote:
  
  > A bit more padding is good!
  >
  > 1. Making this pixel-perfect with every combination of font and font size 
is going to be impossible sadly. With that in mind, the most elegant design is 
one that makes this not a problem, because it doesn't have to be pixel-perfect 
and all the baselines are at least aligned.
  
  
  Yeah...
  
  > 2. No matter what you pick here, it will look wrong in some cases. :(  If 
you anchor the icon to the top left, it will look bad the moment there's a 
second line of text. But if you vertically center it, then it looks bad anytime 
the notification is tall. Judgment call I think, but personally I'm in favor of 
anchoring it to the top left.
  
  Top left sounds good. I'm on the fence to be honest, but I thought to myself 
hey this is a problem for another diff anyway so why not take care of the 
padding first 

INLINE COMMENTS

> hein wrote in NotificationItem.qml:175
> Fractions of smallSpacing can lead to subpixel positioning causing blurry 
> text.

Is there any way around this? Because if it's not 0.75 then it's shifted too 
far down (uneven padding)

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 48711.
rooty added a comment.


  Change shift in height of notification to an integer

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=48709=48711

BRANCH
  notification-padding (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham
Cc: hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  In D17975#386665 , @ngraham wrote:
  
  > Can you clarify what the ultimate visual goal here? Clearly defining that 
might help to tighten up your code and thought process.
  >
  > Also, what remaining issues make this WIP? I find that the Phabricator 
`NOTE:` markup works well for this. See 
https://secure.phabricator.com/book/phabricator/article/remarkup/
  
  
  The basic idea is to get padding all around the notification contents, 
without making the heading stick out too much above the notification item. 
Basically pushing everything slightly inward.
  
  There are two issues that are beyond the scope of this diff though:
  
  - 1) A technical concern: Will it scale properly? Will it handle different 
font sizes properly? At present the notification widget doesn't scale properly 
and I don't know how to fix that so that wasn't my primary concern:
  
  F6525885: image.png 
  
  NOTE: that is an issue present in master (this screenshot is of master, not 
my patch)
  
  
  
  - 2) A design concern: Should the notification icon be lowered in position 
based on the //amount// of content (text) in the notification like what happens 
in certain Windows 10 notifications:
  
  F6525887: image.png 

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: hein, ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 48709.
rooty added a comment.


  Add margins to the right of the buttons on the right border

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=48708=48709

BRANCH
  notification-padding (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty, #vdg, #plasma, ngraham
Cc: ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  Okay, so I figured out how to add padding to the buttons as well, by adding a 
right margin to the mainLayout part (hence the deleted comment describing how I 
couldn't figure it out, my bad):
  
  F6525782: before-n-after.png 

REPOSITORY
  R120 Plasma Workspace

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

To: rooty, #vdg, #plasma, ngraham
Cc: ndavis, plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty added a comment.


  Hey so I'd also like for the Open... (image three in each set) button to 
shift by one units.smallSpacing to the left.
  The close button does, if I add "rightMargin: units.smallSpacing" into 
ColumnLayout / mainLayout, but the Open... button doesn't.
  
  I just don't know how to go about it. Anyone have any idea how to do this?
  Is this worth changing or should the "Open..." (and other buttons like "Got 
it" etc.) stay where it is?

REPOSITORY
  R120 Plasma Workspace

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

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty updated this revision to Diff 48708.
rooty added a comment.


  Correct spelling

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17975?vs=48707=48708

BRANCH
  notification-padding (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17975: [WIP, Notifications] Add padding to notifications

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch aims to add padding to the notification widget and fully remedy 
the issues that arise once it is applied.

TEST PLAN
  [Coming soon]

REPOSITORY
  R120 Plasma Workspace

BRANCH
  notification-padding (branched from master)

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

AFFECTED FILES
  applets/notifications/package/contents/ui/NotificationItem.qml

To: rooty
Cc: plasma-devel, kvanton, jraleigh, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17934: [Kickoff] Increase Kickoff information label readability

[Root]

rooty updated this revision to Diff 48644.
rooty added a comment.


  Roll back vertical alignment change

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17934?vs=48578=48644

BRANCH
  centered-infolabel (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty, ngraham, #vdg, #plasma
Cc: abetts, GB_2, plasma-devel, jraleigh, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D17023: [Kickoff] Change search label and username font size

[Root]

rooty added a comment.


  Abandoned because of parent (D16988 ).

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, romangg, ngraham
Cc: mart, filipf, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D17022: [Kickoff] Enable Esc to close Kickoff and Tab to switch to Search

[Root]

rooty abandoned this revision.
rooty added a comment.


  Abandoned because of parent (D16988 ).

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, ngraham, romangg
Cc: filipf, abetts, davidedmundson, plasma-devel, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D17935: [Clipboard] Use ellipsis in the search field label ('Search...' instead of 'Search')

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch adds the ellipsis (...) to the search field label in Clipboard so 
as to make it consistent with the labels in Kickoff, Kicker and KRunner.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  clipboard-search-ellipsis (branched from master)

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

AFFECTED FILES
  applets/clipboard/contents/ui/ClipboardPage.qml

To: rooty
Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17023: [Kickoff] Change search label and username font size

[Root]

rooty abandoned this revision.
rooty added a comment.


  Abandoned because of parent (D16988 ).

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, romangg, ngraham
Cc: mart, filipf, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D17934: [Kickoff] Increase Kickoff information label readability

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch vertically centers the information label (infolabel) that appears 
once a user hovers/runs over their username with
  their mouse as well as increases its opacity in favor of better readability.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  centered-infolabel (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty
Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17022: [Kickoff] Enable Esc to close Kickoff and Tab to switch to Search

[Root]

rooty added a comment.


  Abandoned because of parent (D16988 ).

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, ngraham, romangg
Cc: filipf, abetts, davidedmundson, plasma-devel, GB_2, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty abandoned this revision.
rooty added a comment.


  Seeing as this causes the latest plasmashell to lock up and I was never too 
enthusiastic about the change either, I'm going to close this, sorry guys.

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson, mart
Cc: mart, abetts, gladhorn, chempfling, filipf, plasma-devel, GB_2, ragreen, 
Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D17020: [Kickoff] Reenable hover to show system information

[Root]

rooty abandoned this revision.
rooty added a comment.


  Abandoned because of parent (D16988 ).

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, ngraham, romangg
Cc: filipf, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17916: [Login and lock screens] Do not use light font styles for the sddm clock

[Root]

rooty updated this revision to Diff 48534.
rooty added a comment.


  Apply patch to login theme as well

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17916?vs=48531=48534

BRANCH
  normal-fonts-lckscrclock (branched from master)

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

AFFECTED FILES
  lookandfeel/contents/components/Clock.qml
  sddm-theme/Clock.qml

To: rooty, ngraham, #vdg, #plasma
Cc: filipf, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17916: [Login and lock screens] Do not use light font styles for the sddm clock

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch removes the line of code that required that the lock screen use a 
light font style.
  This patch is also closely related to the fixes in D17905 
, D17906 
 and D17907 
.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  normal-fonts-lckscrclock (branched from master)

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

AFFECTED FILES
  lookandfeel/contents/components/Clock.qml

To: rooty
Cc: plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17598: Improve "No internet connection!" error message

[Root]

rooty added a comment.


  it might work on the left, but i think this is better because it takes up the 
entire pane, and is important enough to warrant doing so
  
  for example,
  F6517329: image.png 
  F6517334: image.png 
  
  versus firefox's 
  F6517331: image.png 
  (takes up more room)
  
  in contrast to less 'important' prompts such as
  
  F6517338: image.png 
  F6517341: image.png 
  F6517343: image.png 

REPOSITORY
  R134 Discover Software Store

BRANCH
  better-no-connectivity-error-message (branched from master)

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

To: ngraham, #discover_software_store, #vdg, apol, shubham
Cc: rooty, shubham, pino, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17598: Improve "No internet connection!" error message

[Root]

rooty added a comment.


  loving the icon, it really stresses the importance of checking whether your 
internet connection's working without being (too) imposing

REPOSITORY
  R134 Discover Software Store

BRANCH
  better-no-connectivity-error-message (branched from master)

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

To: ngraham, #discover_software_store, #vdg, apol, shubham
Cc: rooty, shubham, pino, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17615: [FolderView] Fix blurry icon labels

[Root]

rooty added a comment.


  my bad, i just didn't copy the entire contents of the 
plasma-desktop/containments/desktop/package/contents/ui/code folder off of 
github (just the FolderItemDelegate.qml), it works fine now that it's all up to 
date

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, #plasma
Cc: rooty, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17615: [FolderView] Fix blurry icon labels

[Root]

rooty added a comment.


  yup i rename it and it turns all blurry, irrespective of where the panel is 
or how much the icons have been selected / moved around

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, #plasma
Cc: rooty, plasma-devel, GB_2, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Root]

rooty added a comment.


  > That's not a feature we have; the wallpapers of the lock screen, login 
screen, and desktops are all independent. Whether or not this is desirable is 
an entirely different issue, but operating with the limitation of not having 
that, there is no point to a "mild blur" on the login screen because it's not a 
hint of anything else; it's just a blurry image.
  
  I actually toyed with this idea by introducing ImageSplash (instead of the 
KDE splash screen) and making the transition from login screen to desktop seem 
seamless (no interruption between login and desktop). Also, if you'd pick a 
different user, the login screen would just fade out and switch to that user's 
wallpaper (again, very tastefully). Unfortunately Kwin (I think it was Kwin) 
likes to flicker on my computer (but not always...) right before the panel pops 
up, so... it wasn't so much seamless as //almost seamless//.

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: raddison, romangg, davidedmundson, rizzitello, abetts, pstefan, broulik, 
rikmills, filipf, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Root]

rooty added a comment.


  so i take it you're open to the idea of an unobtrusive type of blur?
  i get that you want to preserve the wallpaper, but sometimes it really does 
get in the way (snow, mountains, clouds, etc.)

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: raddison, romangg, davidedmundson, rizzitello, abetts, pstefan, broulik, 
rikmills, filipf, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Root]

rooty added a comment.


  In D16031#368991 , @ngraham wrote:
  
  > You have two options:
  >
  > - Stop insulting people and their work
  > - Go away
  
  
  naaate you're taking all of this way too personally
  first of all,, @raddison's got a valid point, these changes to remove the 
blur introduce a lot of new problems (the blur made sure that the text on the 
screen was always legible regardless of the wallpaper chosen (and how much it 
got butchered))
  
  he also touched on a very interesting argument you seem to invoke as 
justification for these changes, i.e. //people have complained//. i can't help 
but wonder if the idea to remove the blur came from somewhere else instead:
  
  F6453429: image.png 

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: raddison, romangg, davidedmundson, rizzitello, abetts, pstefan, broulik, 
rikmills, filipf, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  In D16988#367971 , @filipf wrote:
  
  > In D16988#367970 , @rooty wrote:
  >
  > > Suffice it to say, you can't use any of the other patches (except the 
font size / label change) without this one.
  >
  >
  > Separating this doesn't make sense either because there's no need to change 
"Search..." to "Type to search..." if the field will remain focused as it is 
now.
  
  
  good point. so they're entirely dependent.

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson, mart
Cc: mart, abetts, gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  In D16988#366146 , @ngraham wrote:
  
  > D17020 , D17022 
, and D17023 
 still include duplicated changes from this 
patch.
  >
  > The idea was to make all of your Kickoff patches independent of each other, 
or at least to thin out the dependency tree so that all the patches depend on a 
single ancestor patch that only has uncontroversial refactoring changes. Right 
now the dependency on this one means that all of your other 
much-less-controversial changes are gated on getting acceptance for making the 
search field visually unfocused by default.
  >
  > Am I making any sense?
  
  
  No. What you're saying makes a kind of sense until you actually think about 
it - while it's possible to change D17023  
to make it more discrete (it's a font size and label change, basically three 
lines of code), even that makes little sense in the face of a focused search 
field that would be present without all the other patches. It's not possible to 
make D17022  or D17020 
 (//**re**//-enable hover) independent 
because they //quite literally// depend on an unfocused search field and are 
bug fixes for issues that this here diff introduces. So it makes //very little 
if any sense// to make them independent (the current Kickoff doesn't have these 
bugs).
  
  Suffice it to say, you can't use any of the other patches (except the font 
size / label change) without this one.
  
  P.S. If you want to reject the patch based on controversy, that's fine with 
me. I don't care much about the focus anyway tbh

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson, mart
Cc: mart, abetts, gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol

D16879: [Login and Lock screens] Improve UI elements' contrast a bit

[Root]

rooty added a comment.


  In D16879#362527 , @ngraham wrote:
  
  > IMHO spread 0.1 doesn't provide enough contrast against the toughest 
backgrounds:
  >
  > F6432627: Screenshot_20181119_124804.png 

  
  
  ouch you're right that //is// bad

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, davidedmundson, #vdg, #plasma
Cc: rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17023: [Kickoff] Change search label and username font size

[Root]

rooty updated this revision to Diff 45869.
rooty added a comment.


  - Remove (duplicated) code shared with D17020 
 and D17022 


REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17023?vs=45826=45869

BRANCH
  kickoff-visible-and-unfocused-fonts-type (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty, romangg, ngraham
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17022: [Kickoff] Enable Esc to close Kickoff and Tab to switch to Search

[Root]

rooty updated this revision to Diff 45868.
rooty added a comment.


  - Remove duplicated code common to D17020 
, D17022 
 and D17023 


REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17022?vs=45825=45868

BRANCH
  kickoff-visible-and-unfocused-tab-esc (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty, ngraham, romangg
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17022: [Kickoff] Enable Esc to close Kickoff and Tab to switch to Search

[Root]

rooty added a comment.


  In D17022#362813 , @davidedmundson 
wrote:
  
  > I appreciate the effort to split up a big commit, but this isn't quite 
right.
  >
  > There's duplicated code changes both here and in 
https://phabricator.kde.org/D17020
  
  
  AH now I get it, they ought to be designed as //patches// for D16988 
 as though it were master. My bad.

INLINE COMMENTS

> davidedmundson wrote in Header.qml:285
> it's very unusual to not move the actual focus on tab.

Sorry about that - I'm totally new to this so the only way I could figure out 
how to keep activeFocusOnTab: false in hintLabel was to use this Keys.onPressed 
--> query thing.

I placed activeFocusOnTab: false in hintLabel because the tab key kept 
selecting the label instead of the search box (it did not look pretty).

Using activeFocusOnTab in queryField doesn't seem to do anything once 
activeFocusOnTab: false has been set in hintLabel.

Do you know of any other way to resolve this?

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, ngraham, romangg
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17034: [Kickoff] Fix bad kerning in tab labels

[Root]

rooty added a comment.


  In D17034#362808 , @broulik wrote:
  
  > > The notifications widget also forces NativeRendering in one of its 
components rather than leave it open for discussion.
  >
  > It does because it's using a `TextEdit` directly to make text selectable 
rather than a full `TextArea` which would set this automatically internally.
  
  
  Yeah but it looks like it needs an attitude adjustment. Wouldn't sacrificing 
selectability be acceptable in this case?

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, ngraham, #vdg, #plasma, hein
Cc: hein, broulik, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17034: [Kickoff] Fix bad kerning in tab labels

[Root]

rooty added a comment.


  In D17034#362786 , @hein wrote:
  
  > This isn't an acceptable patch, sorry - we can't just randomly sprinkle a 
different font rasterizer into the UI. The rasterization should be consistent 
throughout the system and consistent with applications, where FreeType (i.e. 
NativeRendering) is the default. Qt's SDF rasterizer (QtRendering) is also 
low-quality and unsuitable for many type faces.
  
  
  The notifications widget also forces NativeRendering in one of its components 
rather than leave it open for discussion. Inconsistency regarding font 
rendering is already a foregone conclusion. Unfortunately, however consistent 
NativeRendering may be, it just //doesn't pass muster//. (This contributes to 
why designers and book publishers prefer mac over windows).
  Also it's hardly random, it's specifically targeted at a label or a 
//single// portion of the UI that's affected by this consistently and gravely.
  
  In D17034#362791 , @hein wrote:
  
  > Thanks, that should probably be reverted, although there might have been 
some reason I'm unaware of in that case since it also sets the font style to 
outline rendering.
  
  
  Why should it be reverted? If it worked without anyone even noticing it did, 
how can this be a problem? If it ain't broke...

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, ngraham, #vdg, #plasma, hein
Cc: hein, broulik, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17034: [Kickoff] Fix bad kerning in tab labels

[Root]

rooty added a comment.


  (LOL I accidentally removed the comment)
  NativeRendering really is the gift that keeps on giving.
  F6432908: nativerendering.png 
  QtRendering renders a lot better.
  F6432910: qtrendering.png 
  
  This is actually pretty neat considering Kickoff and Notifications are the 
only two widgets where this is a noticeable problem.

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, ngraham, #vdg, #plasma
Cc: rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17034: [Kickoff] Fix bad kerning in tab labels

[Root]

rooty added a comment.


  NativeRendering truly is the gift that keeps on giving
  F6432893: nativerendering.png 
  This is what happens if I specify that JobDetailsItem.qml use QtRendering
  F6432899: qtrendering.png 

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, ngraham, #vdg, #plasma
Cc: rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17034: [Kickoff] Fix bad kerning in tab labels

[Root]

rooty added a comment.


  this has been bugging me/us for years, glad you finally decided to patch it :D

REPOSITORY
  R119 Plasma Desktop

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

To: filipf, ngraham, #vdg, #plasma
Cc: rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Root]

rooty added a comment.


  In D16031#362635 , @ngraham wrote:
  
  > In D16031#362634 , @rooty wrote:
  >
  > > Another fair point: Why not just pick an image that doesn't really 
interfere with the boxes and icons that much then you don't need blur at all.
  >
  >
  > Because within reason, it's the system's job to accommodate the user, not 
the reverse.
  
  
  No, I mean, I did phrase it in the 2nd person but I was talking about KDE 
picking a wallpaper that's appropriate to the login environment. One of the 
things that blur does really well is make it idiotproof so wallpaper selection 
isn't as much of an issue (however it does sort of butcher the wallpaper).

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: romangg, davidedmundson, rizzitello, abetts, pstefan, broulik, rikmills, 
filipf, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Root]

rooty added a comment.


  In D16031#362566 , @ngraham wrote:
  
  > Thing is, if we're going to reduce the blur that much, why not just show 
the original image?
  
  
  That's a fair point.
  Another fair point: Why not just pick an image that doesn't really interfere 
with the boxes and icons that much then you don't need blur at all.
  
  That being said, I personally prefer blur - it's just a matter of taste. The 
added bonus with the blur is the "effect" or "animation" (more like, the time 
needed for it to show up and disappear later on). Not to belabor the point (too 
much) but I've also been using ImageSplash (a splash screen that uses an image 
instead of an actual splash screen) to bridge the gap between the login screen 
and the desktop, and you get a very smooth transition from login to desktop 
with both blur and non-blur when it disappears (and my graphics drivers decide 
//not to glitch//), check it out you might like it:
  
  F6432738: phone-recording.mp4 
  
  Might be something worth investigating,

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: romangg, davidedmundson, rizzitello, abetts, pstefan, broulik, rikmills, 
filipf, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D17023: [Kickoff] Depends on D16988, Change search label and username font size

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  Included commits in branch kickoff-visible-and-unfocused-fonts-type:
  78eb4b0d4e08 [Kickoff] Depends on D16988 
, Change search label and username font size

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff-visible-and-unfocused-fonts-type (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17022: [Kickoff] Depends on D16988, Enable Esc to close Kickoff and Tab to switch to Search

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  #Summary:
  
  #Test Plan:
  
  #Subscribers:
  
  1. NEW DIFFERENTIAL REVISION
  2. Describe the changes in this new revision. #
  
  Included commits in branch kickoff-visible-and-unfocused-tab-esc:
  63c6197a151a [Kickoff] Depends on D16988 
, Enable Esc to close Kickoff and Tab to 
switch to Search

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff-visible-and-unfocused-tab-esc (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D17020: [Kickoff] Depends on D16988, Reenable hover to show system information

[Root]

rooty created this revision.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
rooty requested review of this revision.

REVISION SUMMARY
  This patch reenables the mouseover to show system information function 
introduced by Nate Graham and tailors it to D16988 

  
  #Test Plan:
  
  #Reviewers:
  
  #Subscribers:
  
  1. NEW DIFFERENTIAL REVISION
  2. Describe the changes in this new revision. #
  
  Included commits in branch kickoff-visible-and-unfocused-hover:
  #
  5775150fcf26 [Kickoff] Depends on D16988 
, Reenable hover to show system information

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff-visible-and-unfocused-hover (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty updated this revision to Diff 45820.
rooty added a comment.


  - [Kickoff] Make search field unfocused but visible

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16988?vs=45757=45820

BRANCH
  kickoff-visible-and-unfocused (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: abetts, gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  Thanks!
  
  Ironically, I do like it focused too 藍

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: abetts, gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, 
ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  > You can do this as you see fit. I would recommend though in this case to 
make a patch series where patches depend on each other since otherwise it can 
become difficult to keep all the independent patches consistent with each other.
  
  Excellent.
  
  > I believe in you! :) If you need quick help come join the Plasma IRC 
channel.
  
  Thanks! :D I just have to figure out how IRC works now. And I thought //arc// 
was a challenge 藍

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  In D16988#362281 , @romangg wrote:
  
  > I like all the changes, but they should go into separate patches:
  
  
  Patches that would patch this patch or patches that would each patch master 
separately?

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  > Why does it make it easy?
  
  Sorry I have no idea what I was thinking when I wrote that. It doesn't. (I 
think my point was that it works regardless of the focus being drawn away from 
it?)
  
  > - By default search field looks unactivated but has focus. Arrow up/down 
sets focus to the item selected, but typing to start a search is still possible.
  
  Clipboard works like this but the coding is beyond my abilities. Krunner does 
this too except it opens up with the search field in focus. This Kickoff 
patched version can't do that, and I've yet to figure out how to make it be 
able to.
  
  > - One does not need to go back to the search field with arrow keys to 
add/remove characters (and also should not be able to do that like in Kicker).
  
  It's not necessary to do so in Kicker. You can just keep on typing.

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16937: [Kickoff] Make Kickoff separators leaner (less opaque) and narrower (the length of a highlighted menu item)

[Root]

rooty added a comment.


  my real name is Kresimir Cohar, email address: kco...@gmail.com
  thanks you guys :D
  
  i just realized i have a pseudonym on kde identity... i might have to fix that

REPOSITORY
  R119 Plasma Desktop

BRANCH
  kickoff-leaner-separators (branched from master)

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

To: rooty, ngraham, #vdg, romangg
Cc: romangg, filipf, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  In D16988#362206 , @chempfling 
wrote:
  
  > In D16988#362160 , @rooty wrote:
  >
  > > Okay so from what I can tell:
  > >
  > > (1) Kicker takes the focus off the search field when you go through the 
results by either mouse or keyboard. This makes it easy to go back to the 
search field by typing or using the arrow keys (to add more words etc.)
  > >  (2) Krunner becomes unfocused if you use the arrow keys, **but not if 
you hover over the search results with your mouse**, and it's still possible in 
either case to append your search same as in Kickoff.
  > >  (3) Kickoff (the old, the master and this patch) seems to leave the 
search field focused at all times while you go through the results, and it's 
possible to add more characters to the search like in (1) and (2).
  > >  (4) Clipboard behaves pretty much like this patch except //pressing the 
Esc key doesn't make it unfocused again//.
  > >
  > > Any thoughts on how to reconcile all these differences? It might take 
some doing 
  >
  >
  > Howdy,
  >
  > just for clearification :). With focus and unfocuse we talk about "focus: 
true", not only an highlighting effect?
  
  
  Yup - not just a cosmetic effect

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  Okay so from what I can tell:
  
  (1) Kicker takes the focus off the search field when you go through the 
results by either mouse or keyboard. This makes it easy to go back to the 
search field by typing or using the arrow keys (to add more words etc.)
  (2) Krunner becomes unfocused if you use the arrow keys, **but not if you 
hover over the search results with your mouse**, and it's still possible in 
either case to append your search same as in Kickoff.
  (3) Kickoff (the old, the master and this patch) seems to leave the search 
field focused at all times while you go through the results, and it's possible 
to add more characters to the search like in (1) and (2).
  (4) Clipboard behaves pretty much like this patch except //pressing the Esc 
key doesn't make it unfocused again//.
  
  Any thoughts on how to reconcile all these differences? It might take some 
doing 

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  @gladhorn More food for though, by the way: in this patch (and the master) 
the search field is focused while searching and while navigating the search 
results - it still accepts input even though you've technically navigated away 
from it. Should this functionality be kept (regardless of if the search field 
remains visibly focused or not), or should we prevent the user from typing into 
the search field while the focus is on the search results?

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  > Actually this sounds like it might help our cause. Especially when the 
search bar can be tabbed into, that's really nice. The problem is that we would 
like to focus to be on the selected item, such as the one that gets launched 
when pressing enter. The search should not have the focus unless the user type. 
I didn't look at the code change at all, but from the description it may just 
be a step forward.
  
  This did occur to me too, and it's a great idea, I just haven't figured out 
how to implement it yet haha

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16031: [SDDM theme] remove blur and increase UI contrast so that it's not required

[Root]

rooty added a comment.


  People have been complaining about the blur, but is it necessary to do away 
with it, or is it a better idea to run with it like they do in Windows Fluent 
Design (Deepin too actually)?
  F6431816: image.png 
  
  This theme, for example, has different blur that might work (It makes you 
think you need glasses though).
  It's a Breeze-Chili hybrid that incorporates the proposed font changes in 
ActionButton.qml and UserDelegate.qml. I'm just not sure about the shadows 
behind the clock (spread: 0.1) but they look great full screen. It also uses 
Chili's password box (even though I do like the Breeze one, not really sure 
about that). I haven't yet incorporated the latest system.svgz icons here 
though, because they look sort of out of place on top of a blurry background.
  F6431828: image.png 

REPOSITORY
  R120 Plasma Workspace

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

To: ngraham, #vdg, #plasma, rizzitello, davidedmundson
Cc: romangg, davidedmundson, rizzitello, abetts, pstefan, broulik, rikmills, 
filipf, rooty, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  > Right, I agree. In fact, that's how Kicker does it: the search field is 
focused by default, but if you mouse over a list item, then focus visually 
moves to the list. I'm supportive of doing that sort of thing here. Then the 
Kicker and Kickoff search fields would even be consistent with one another, 
which would be a nice benefit.
  
  Not bad. I'd have to rework this Header.qml to make it more like Kicker 
though. But that's only if everyone's really sure they don't want the unfocused 
search field.
  
  By the way, should I separate the font and hover changes into two separate 
patches or just one medium-sized one? :D

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16937: [Kickoff] Make Kickoff separators leaner (less opaque) and narrower (the length of a highlighted menu item)

[Root]

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty added a comment.


  > And I'm afraid I don't think we can do #1. @chempfling and @gladhorn have 
been working hard to push on accessibility, and one thing I've learned in the 
past week weeks is how important focus is. Making sure that the active element 
both has and looks like it has focus is critical to making sure  the UI is 
accessible for screen readers. As such, we need to keep it visually focused by 
default when it opens.
  
  I respectfully disagree. While this is a very valid point (and I don't 
dislike the already focused search field), the verb type in the imperative mood 
is the most efficient way to inform the user that they can search. Furthermore, 
the UI seeming accessible if focused is more of an intuitive leap. Spotlight, 
for instance, does this by means of a magnifying glass, and it moves onto the 
focused search results by default, but it doesn't focus the search field - 
there is //simply little if any benefit// in keeping the actual search field 
focused (it's too obvious). That's why I was tinkering with the idea of 
removing the focus altogether (but I do like the blue).
  
  //In short, there is simply no need to have the search field visually focused 
by default when it opens, not in Kicker (the text would need to be changed to 
Type to search...), not in Krunner and not even in Clipboard (where this has 
already long been implemented). 
  //
  
  > One more note: while compatibility with 3rd-party themes is a laudable 
goal, it can never be a primary one, and it can't dictate design 
considerations. 3rd-party themes are a moving target and chasing them is 
futile, so  "this change makes it look better with 3rd-party themes" can never 
be a selling point. It has to be better for Breeze first and foremost. If the 
change also improves the look for 3rd-party themes, all the better, but that 
can't be the primary consideration.
  
  Futile? I beg to differ. While not primary, it should still be paramount. The 
fact that it makes it look better for third party themes should be a major 
concern for KDE, because KDE is touted as endlessly customizable (and this is 
one of the main reasons a lot of people prefer KDE to GNOME... or Windows/Mac, 
for that matter).
  
  > So I'd like to see the font size changes go into one patch, the keyboard 
navigation improvements go into a second, and the search field placeholder text 
change in a third one. I think those are fairly non-controversial.
  
  While in principle I do agree, this is impossible to accomplish here without 
making a decision about whether the search field is unfocused or not. Splitting 
up the patch means that the keyboard improvements go down the drain //(Esc is 
used to escape no matter what (this works already) and there's no need to use 
Tab to select the search field (it's already in focus)//. The only thing that's 
salvageable is the font size change and the change in the text within the field 
(and the hover font change). The hover changes themselves (not the font), the 
keyboard improvements etc. are simply too intertwined with the unfocused search 
field and the way the states are defined. You'd have to rewrite the unfocused 
portions, but there's essentially no point in doing so seeing as you'll never 
be using those keyboard improvements in the first place if it's already focused.

REPOSITORY
  R119 Plasma Desktop

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

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: gladhorn, chempfling, filipf, plasma-devel, ragreen, Pitel, ZrenBot, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty updated this revision to Diff 45757.
rooty added a comment.


  - Trim unnecessary code

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16988?vs=45753=45757

BRANCH
  kickoff-visible-and-unfocused (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: filipf, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty updated this revision to Diff 45753.
rooty added a comment.


  - Fix broken states and return ability to hover

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16988?vs=45751=45753

BRANCH
  kickoff-visible-and-unfocused (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart

D16988: [Kickoff] Make the visible search field unfocused by default

[Root]

rooty updated this revision to Diff 45751.
rooty added a comment.


  - Tidy up some more

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16988?vs=45750=45751

BRANCH
  kickoff-visible-and-unfocused (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/ui/Header.qml

To: rooty, #plasma, #vdg, romangg, ngraham, davidedmundson
Cc: plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart