D25398: Check values have changed before setting the model

[Méven Car]

meven added a comment.


  In D25398#564887 , @ngraham wrote:
  
  > In D25398#564843 , @meven wrote:
  >
  > > I had a look at the others :
  > >
  > > > - https://bugs.kde.org/show_bug.cgi?id=392943
  > > > - https://bugs.kde.org/show_bug.cgi?id=389878
  > > > - https://bugs.kde.org/show_bug.cgi?id=393702
  > >
  > > Something I noticed when having an avatar and setting any other parameter.
  > >  Apparently AccountService removes the icon as it considered it transient 
cache data :
  > >  
https://github.com/magcius/accountsservice/blob/eb166ec7592704a897594f7ca4c47b70d871a85c/src/daemon.c#L1105
  >
  >
  > So it's an upstream issue? Is that really where upstream AccountsService 
lives? Some random guy's GitHub?
  
  
  This was a clone of an old version.
  
  Current behavior is the same in the current version.
  https://cgit.freedesktop.org/accountsservice/tree/src/daemon.c#n155
  Apparently the accountservice daemon wants to uncache the user and in doing 
so delete data without updating its internal reference.
  
  GNOME does the same as our user-manager but I guess it is not affected.
  
https://github.com/GNOME/gnome-control-center/blob/f5f67823db5a629fa3ce618bc1d8b80d14f1fc59/panels/user-accounts/cc-avatar-chooser.c#L87
  
  I did some digging but could not pinpoint the problem origin.
  
  In the meantime, I believe this diff is worth merging and might be useful for 
stable branch even.

REPOSITORY
  R128 User Manager

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

To: meven, #plasma
Cc: ngraham, apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D12916: Panel should not stop auto-hiding if a window wants attention

[Michael Moon]

michaelmoon updated this revision to Diff 70036.
michaelmoon added a comment.


  Updated for 5.16.5, which the previous patch failed against.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D12916?vs=61838&id=70036

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

AFFECTED FILES
  applets/taskmanager/package/contents/config/main.xml
  applets/taskmanager/package/contents/ui/ConfigBehavior.qml
  applets/taskmanager/package/contents/ui/main.qml

To: michaelmoon, ngraham, hein
Cc: hein, ngraham, davidedmundson, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25006: Make sure -dpi value is valid

[Filip Fila]

filipf added a comment.


  Friendly ping.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

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

To: gcraciunescu, filipf, #plasma, ngraham, davidedmundson
Cc: davidedmundson, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25415: Improve package documentation

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  LGTM.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  better-documentation (branched from master)

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

To: filipf, #plasma, #vdg, ngraham
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25413: [Weather] Make compact representation temperature text match size of clock text

[Nathaniel Graham]

ngraham added a comment.


  Sure, if and when this lands, I'll add that note to the applicable place in 
the code for the Digital Clock. And then we can plan out how to turn this into 
a constant that lives somewhere else.

REPOSITORY
  R114 Plasma Addons

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

To: ngraham, #vdg, #plasma
Cc: filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25418: [Folder view] Use selected text color for selected items in full representation

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: VDG, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Right selected items persist in using the default text color , which can 
result in
  readability problems for light themes that use a dark highlight effect.
  
  BUG: 414287
  FIXED-IN: 5.17.4

TEST PLAN
  Before: F7772272: Before.png 
  
  After: F7772271: After.png 

REPOSITORY
  R119 Plasma Desktop

BRANCH
  fix-folderview-selected-text (branched from Plasma/5.17)

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

AFFECTED FILES
  containments/desktop/package/contents/ui/FolderItemDelegate.qml

To: ngraham, #vdg, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25413: [Weather] Make compact representation temperature text match size of clock text

[Filip Fila]

filipf added a comment.


  > and I should feel bad.
  
  :D
  
  +1. It might also be useful to add a little reminder in the digital clock's 
code that the magic numbers are also used here if someone wants to change 
digital clock.

REPOSITORY
  R114 Plasma Addons

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

To: ngraham, #vdg, #plasma
Cc: filipf, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25024: Use the same avatar size in lock, login and logout screens

[Filip Fila]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:f56e800c9634: Use the same avatar size in lock, login and 
logout screens (authored by filipf).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25024?vs=68928&id=70021

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

AFFECTED FILES
  lookandfeel/contents/components/UserDelegate.qml
  lookandfeel/contents/logout/Logout.qml

To: filipf, #plasma, #vdg, ngraham
Cc: davidedmundson, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, 
jraleigh, fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25415: Improve package documentation

[Filip Fila]

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

REVISION SUMMARY
  The old readme file was just 4 outdated lines on how to compile the package..
  
  I've now tried to add all the crucial info and write the file in Markdown.

REPOSITORY
  R123 SDDM Configuration Panel (KCM)

BRANCH
  better-documentation (branched from master)

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

AFFECTED FILES
  README
  README.md

To: filipf
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25379: WIP: Beginning of the work for Simplifying the Emoticon KCM

[Tomaz Canabrava]

tcanabrava updated this revision to Diff 70018.
tcanabrava added a comment.


  - Fix speed of the models
  - Simplify Logic
  - Speed up emoticon loading
  - Re-Enable updateBtn
  - Re-enable remove theme
  - Move more data to the Models
  - Re-enable the rest of the functions
  - Remove ui file

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25379?vs=69952&id=70018

BRANCH
  kcm_emoticon

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

AFFECTED FILES
  kcms/emoticons/CMakeLists.txt
  kcms/emoticons/editdialog.cpp
  kcms/emoticons/editdialog.h
  kcms/emoticons/emoticonslist.cpp
  kcms/emoticons/emoticonslist.h
  kcms/emoticons/emoticonslist.ui
  kcms/emoticons/emoticonsmodel.cpp
  kcms/emoticons/emoticonsmodel.h
  kcms/emoticons/emoticonthemesmodel.cpp
  kcms/emoticons/emoticonthemesmodel.h
  kcms/emoticons/helpers.cpp
  kcms/emoticons/helpers.h

To: tcanabrava
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25398: Check values have changed before setting the model

[Nathaniel Graham]

ngraham added a comment.


  In D25398#564843 , @meven wrote:
  
  > I had a look at the others :
  >
  > > - https://bugs.kde.org/show_bug.cgi?id=392943
  > > - https://bugs.kde.org/show_bug.cgi?id=389878
  > > - https://bugs.kde.org/show_bug.cgi?id=393702
  >
  > Something I noticed when having an avatar and setting any other parameter.
  >  Apparently AccountService removes the icon as it considered it transient 
cache data :
  >  
https://github.com/magcius/accountsservice/blob/eb166ec7592704a897594f7ca4c47b70d871a85c/src/daemon.c#L1105
  
  
  So it's an upstream issue? Is that really where upstream AccountsService 
lives? Some random guy's GitHub?

REPOSITORY
  R128 User Manager

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

To: meven, #plasma
Cc: ngraham, apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D25413: [Weather] Make compact representation temperature text match size of clock text

[Nathaniel Graham]

ngraham created this revision.
ngraham added reviewers: VDG, Plasma.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Right now, the Weather widget's compact representation's temperature text is 
not the same
  size as the text for the clock, which looks especially awkward when both are 
placed
  adjacent to one another. In addition, the text size has no upper bound, so it 
can become
  absolutely gigantic with a tall panel.
  
  This patch fixes both issues by using the same size logic used in the digital 
clock,
  ensuring that the text sizes are identical.
  
  I know that this uses a magic value, and that this is bad and I should feel 
bad.
  However it's the same one used in the digital clock. Once more than one 
widget is using
  this "height * 0.7" value, it becomes feasible to factor it out into 
something in
  plasma-framework maybe so we can use it to get consistent text size 
everywhere.
  
  Hopefully that can be material for a subsequent patch though.

TEST PLAN
  No change for vertical panels.
  
  Before, default horizontal panel: F7772047: Before, small panel.png 

  
  After, default horizontal panel: F7772044: After, small panel.png 

  
  Before: double-height horizontal panel: F7772046: Before, double-height 
panel.png 
  
  After, double-height horizontal panel: F7772045: After, double-height 
panel.png 
  
  Before, absurdly humongous horizontal panel: F7772048: Before, humongously 
enormous panel.png 
  
  After, absurdely humongous horizontal panel: F7772049: After, humongously 
enormous panel.png 

REPOSITORY
  R114 Plasma Addons

BRANCH
  fix-weather-widget-temperature-text-size

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

AFFECTED FILES
  applets/weather/package/contents/ui/IconAndTextItem.qml

To: ngraham, #vdg, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25398: Check values have changed before setting the model

[Méven Car]

meven added a comment.


  In D25398#564706 , @ngraham wrote:
  
  > Nice. Does this fix any of these?
  >
  > - https://bugs.kde.org/show_bug.cgi?id=384894
  
  
  This diff fix this one
  
  I had a look at the others :
  
  > - https://bugs.kde.org/show_bug.cgi?id=392943
  > - https://bugs.kde.org/show_bug.cgi?id=389878
  > - https://bugs.kde.org/show_bug.cgi?id=393702
  
  Something I noticed when having an avatar and setting any other parameter.
  Apparently AccountService removes the icon as it considered it transient 
cache data :
  
https://github.com/magcius/accountsservice/blob/eb166ec7592704a897594f7ca4c47b70d871a85c/src/daemon.c#L1101

REPOSITORY
  R128 User Manager

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

To: meven, #plasma
Cc: ngraham, apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D25408: [KCM] Port main page to QQC2

[Nathaniel Graham]

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

REVISION SUMMARY
  BUG: 414211
  FIXED-IN: 5.18.0

TEST PLAN
  Open Network Connections KCM and use it; no visual or behavioral regressions

REPOSITORY
  R116 Plasma Network Management Applet

BRANCH
  port-to-qqc2 (branched from master)

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

AFFECTED FILES
  kcm/qml/main.qml

To: ngraham, jgrulich
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25395: Make avatar icon dpr aware aka HiDPI rendered

[Méven Car]

meven updated this revision to Diff 70008.
meven added a comment.


  Code style

REPOSITORY
  R128 User Manager

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25395?vs=69982&id=70008

BRANCH
  arcpatch-D25395

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

AFFECTED FILES
  src/accountinfo.cpp
  src/lib/accountmodel.cpp
  src/lib/accountmodel.h

To: meven, #plasma
Cc: crossi, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Michail Vourlakos]

mvourlakos added a comment.


  Nice! I will include it also for upcoming Latte v,0. 9.5

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: ognarb, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Carl Schwan]

This revision was automatically updated to reflect the committed changes.
Closed by commit R878:a21f46d85ff8: [414174] - Panel background transition 
color bug (authored by acemond, committed by ognarb).

REPOSITORY
  R878 Latte Dock

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25392?vs=69977&id=70004

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

AFFECTED FILES
  containment/package/contents/ui/PanelBox.qml

To: acemond, mvourlakos
Cc: ognarb, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Adrien Brunelat]

acemond added a comment.


  But maybe you meant you needed them here.
  author: Adrien Brunelat
  email: adrien.brune...@gmail.com

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: ognarb, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25406: [ToolTip] Base timeout on text length

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.
This revision is now accepted and ready to land.


  Ah so it's in Qt itself and not something we do? Oh well, best to at least be 
consistent, then.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: broulik, #plasma, ngraham, #vdg
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25406: [ToolTip] Base timeout on text length

[Kai Uwe Broulik]

broulik added a comment.


  >   I would prefer that we use a hardcoded timeout and them change the 
QWidgets tooltips duration to follow that.
  
  That requires a behavior change in Qt to something that has been like this 
for decades.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: broulik, #plasma, ngraham, #vdg
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25406: [ToolTip] Base timeout on text length

[Nathaniel Graham]

ngraham added a reviewer: VDG.
ngraham added a comment.


  +1 for fixing the bug
  
  -1 for making it use a length-based calculation. IMO this is a case of being 
"so smart it's dumb" because it cannot predict user reading speed, take into 
account non-word-length differences due to translation, etc. Also, it 
encourages developers to put way too much text in a tooltip. If you can't read 
a tooltip's text in five or seven seconds, it's too long. I would prefer that 
we use a hardcoded timeout and them change the QWidgets tooltips duration to 
follow that.

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

To: broulik, #plasma, ngraham, #vdg
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Adrien Brunelat]

acemond added a comment.


  @ognarb ok I'll look into that. Maybe I should read the instructions or 
tutorials provided on Phabricator. For now I only added them to my profile.

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: ognarb, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25406: [ToolTip] Base timeout on text length

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, ngraham.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  This mimics `QToolTip` behavior and ensures the tooltip is actually readable. 
The property naming is quite confusing;
  
  - `delay` is the delay until the tooltip shows when `visible` becomes `true`
  - `timeout` is the timeout until the tooltip hides //regardless of// whether 
`visible` is still `true`! It is //not// a delay after `visible` becomes false

TEST PLAN
  - KScreen rotation button tooltips are readable now

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

AFFECTED FILES
  org.kde.desktop/ToolTip.qml

To: broulik, #plasma, ngraham
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Carl Schwan]

ognarb added a comment.


  @acemond you need to wait for @mvourlakos to commit the change. You should 
also provide your authorship information name and email address.

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: ognarb, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Adrien Brunelat]

acemond added a comment.


  I added myself to the project but I still don't see any way to commit. Maybe 
I should have used `arc` from the beginning?

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Adrien Brunelat]

acemond added a comment.


  In D25392#564700 , @mvourlakos 
wrote:
  
  > do you have commit access?
  
  
  I just registered to Phabricator so I'm pretty sure that if you feel the need 
to ask me that, then the answer is probably no :)

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25404: Just return applicationDisplayName in MPRIS Identity

[Kai Uwe Broulik]

broulik updated this revision to Diff 7.
broulik added a comment.


  - Drop now unused include

REPOSITORY
  R856 Plasma Browser Integration

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25404?vs=6&id=7

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

AFFECTED FILES
  host/mprisplugin.cpp

To: broulik, #plasma, fvogt
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25404: Just return applicationDisplayName in MPRIS Identity

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, fvogt.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  Avoids some code duplication.

TEST PLAN
  "Firefox Web Browser" is now "Mozilla Firefox"

REPOSITORY
  R856 Plasma Browser Integration

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

AFFECTED FILES
  host/mprisplugin.cpp

To: broulik, #plasma, fvogt
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25403: Drop Position change signal and support Seeked signal

[Kai Uwe Broulik]

broulik created this revision.
broulik added reviewers: Plasma, fvogt.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
broulik requested review of this revision.

REVISION SUMMARY
  When user seeks, emit `Seeked` signal. The signal is periodically emitted 
while seeking and once again when seeking finishes (user releases mouse).
  The `Position` property explicitly isn't signalled as per specification.

TEST PLAN
  - Moved the seek slider in a video, got the `Seeked` signal emitted every 
250ms, and once again when mouse got released.
  - Position of slider in media controller applets seems consistent

REPOSITORY
  R856 Plasma Browser Integration

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

AFFECTED FILES
  host/mprisplugin.cpp
  host/mprisplugin.h

To: broulik, #plasma, fvogt
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25381: Make QPushButton/QToolButton the same height as QLineEdit

[Yunhe Guo]

guoyunhe added a comment.


  Left: JuK (Qt Widgets), Right: Elisa (Qt QML)
  
  F7771662: image.png 

REPOSITORY
  R31 Breeze

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

To: guoyunhe, #breeze, #plasma
Cc: ndavis, GB_2, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25381: Make QPushButton/QToolButton the same height as QLineEdit

[Nathaniel Graham]

ngraham added a comment.


  If this patch makes the sizes match those in Kirigami/QML apps, I guess that 
seems fine, since nobody has noticed any problems with the sized there, right?

REPOSITORY
  R31 Breeze

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

To: guoyunhe, #breeze, #plasma
Cc: ndavis, GB_2, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25398: Check values have changed before setting the model

[Nathaniel Graham]

ngraham added a comment.


  Nice. Does this fix any of these?
  
  - https://bugs.kde.org/show_bug.cgi?id=384894
  - https://bugs.kde.org/show_bug.cgi?id=392943
  - https://bugs.kde.org/show_bug.cgi?id=389878
  - https://bugs.kde.org/show_bug.cgi?id=393702

REPOSITORY
  R128 User Manager

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

To: meven, #plasma
Cc: ngraham, apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, GB_2, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, ahiemstra, mart

D25395: Make avatar icon dpr aware aka HiDPI rendered

[Cyril Rossi]

crossi added inline comments.

INLINE COMMENTS

> accountmodel.cpp:545
> +void AccountModel::setDpr(qreal dpr) {
> +this->dpr = dpr;
> +}

See my previous comment, 'this->' prefix won't be necessary.

> accountmodel.h:95
>  AutomaticLoginSettings m_autoLoginSettings;
> +qreal dpr = 1;
>  };

Style related, attribute member are prefixed with 'm_', see above declarations.

REPOSITORY
  R128 User Manager

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

To: meven, #plasma
Cc: crossi, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Michail Vourlakos]

mvourlakos accepted this revision.
mvourlakos added a comment.
This revision is now accepted and ready to land.


  @acemond forget my previous comment... background transition works much 
better with your commit...
  do you have commit access?

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Michail Vourlakos]

mvourlakos added a comment.


  The second behavior shouldn't be needed, it is already covered by the first 
one

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25398: Check values have changed before setting the model

[Méven Car]

meven added a comment.


  In D25398#564639 , @apol wrote:
  
  > The commit message looks wrong, maybe you meant PolicyKit? PackageKit 
should be irrelevant.
  
  
  Thank you for pointing it out.
  
  > Other than that +1
  
  Do you mean to accept it then ?

REPOSITORY
  R128 User Manager

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

To: meven, #plasma
Cc: apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, ahiemstra, mart

D25398: Check values have changed before setting the model

[Aleix Pol Gonzalez]

apol added a comment.


  The commit message looks wrong, maybe you meant PolicyKit? PackageKit should 
be irrelevant.
  
  Other than that +1

REPOSITORY
  R128 User Manager

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

To: meven, #plasma
Cc: apol, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, ahiemstra, mart

D25381: Make QPushButton/QToolButton the same height as QLineEdit

[Yunhe Guo]

guoyunhe added a comment.


  In QML applications, buttons and line edits are already same height, the same 
height as QLineEdit in Qt Widgets applications. If we increase the height of 
QLineEdit, we have to change QML components' height, too. The amount of work 
will be bigger. If a view contains a lot of QLineEdit/QComboBox rows (like many 
KCM views), the content might overflow. So it is safer to make button height 
shorter.

REPOSITORY
  R31 Breeze

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

To: guoyunhe, #breeze, #plasma
Cc: ndavis, GB_2, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25398: Check values have changed before setting the model

[Méven Car]

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

REVISION SUMMARY
  Only save data that needs to be saved and use sanitized data rather than raw 
text field input.

TEST PLAN
  Edit Username -> have a KPackageKit password verification
  Edit Avatar -> no more password verification popup

REPOSITORY
  R128 User Manager

BRANCH
  master

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

AFFECTED FILES
  src/accountinfo.cpp

To: meven, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25385: Use icon name instead of pixmap in notification

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R116:fc443ccedbfc: Use icon name instead of pixmap in 
notification (authored by nicolasfella).

REPOSITORY
  R116 Plasma Network Management Applet

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25385?vs=69964&id=69985

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

AFFECTED FILES
  kded/notification.cpp
  libs/editor/connectioneditorbase.cpp
  libs/handler.cpp

To: nicolasfella, #plasma, jgrulich, ngraham
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25384: Remove some unused includes

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:caaedab8808e: Remove some unused includes (authored by 
nicolasfella).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25384?vs=69962&id=69984

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

AFFECTED FILES
  kcms/kfontinst/kcmfontinst/DuplicatesDialog.cpp

To: nicolasfella, #plasma, apol
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25383: Remove some unused includes

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R97:3729eb68013e: Remove some unused includes (authored by 
nicolasfella).

REPOSITORY
  R97 Bluedevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25383?vs=69961&id=69983

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

AFFECTED FILES
  src/wizard/pages/discover.cpp
  src/wizard/pages/pairing.cpp

To: nicolasfella, #plasma, drosca
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25395: Make avatar icon dpr aware aka HiDPI rendered

[Méven Car]

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

REPOSITORY
  R128 User Manager

BRANCH
  master

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

AFFECTED FILES
  src/accountinfo.cpp
  src/lib/accountmodel.cpp
  src/lib/accountmodel.h

To: meven, #plasma
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Adrien Brunelat]

acemond added a comment.


  Also, leaving the background set to transparent piece of code AND adding the 
animation would speed up the disappearance of the panel (in case of hide bg 
when not needed) and the animation for fade in and fade out would not have the 
same duration.

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Adrien Brunelat]

acemond added a comment.


  There is no need to set BG to transparent if "busyBackground || coloredView" 
is false, because in this case, opacity will be at 0.
  BG transition must be smooth when colorizerManager changes color though, so 
it needs its separate animation. (transition from plasma reverse to window 
color for instance)

REPOSITORY
  R878 Latte Dock

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

To: acemond, mvourlakos
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25392: [414174] - Panel background transition color bug

[Adrien Brunelat]

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

REVISION SUMMARY
  Bug reported: https://bugs.kde.org/show_bug.cgi?id=414174
  
  Added animation on background color change.
  Removed test for background color that is not needed any more.

REPOSITORY
  R878 Latte Dock

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

AFFECTED FILES
  containment/package/contents/ui/PanelBox.qml

To: acemond, mvourlakos
Cc: plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, GB_2, 
ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25381: Make QPushButton/QToolButton the same height as QLineEdit

[Noah Davis]

ndavis added a comment.


  In D25381#564512 , @guoyunhe wrote:
  
  > In D25381#564404 , @ndavis wrote:
  >
  > > If we do make flat toolbuttons buttons (`autoRaise == true`) and non-flat 
buttons the same size, we will need to change the default icon size for all 
buttons to 22px, or we will have a ton of UI regressions. Any non-flat button 
with a fixed icon size is going to have a regression. Anyone who was using 
non-flat buttons with 22px icons to get a bigger clickable area will need to 
switch to 32px icons. This also means that breeze-icons' problem with 32px icon 
style consistency will show itself more often.
  >
  >
  > Thanks for the remind!
  >
  > By default, QToolButton with 22px icons look good:
  >
  > F7771214: image.png 
  >
  > When change to 32px, still okay:
  >
  > F7771218: image.png 
  >
  > Maybe this is because Dolphin has all icons in 32px. Do you know any 
application that miss 32px icons in toolbar? Thanks!
  
  
  This issue is most likely to show up in 3rd party apps. Maybe we shouldn't 
let an issue like that dictate the design of Breeze widgets though since I 
expect either Breeze icons or the XDG icon specs will change one day to fix 
that issue. For now, I think we should stick to reducing the size of non-flat 
buttons or increasing the size of comboboxes and line edits. If we make 
non-flat button sizes match flat button sizes, we should do that in another 
patch.
  
  I personally think the height of line edits and comboboxes should be 
increased to 32px. This also makes it so we won't have to change how flat 
button sizes work if/when the non-flat buttons are made to match them.

REPOSITORY
  R31 Breeze

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

To: guoyunhe, #breeze, #plasma
Cc: ndavis, GB_2, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25381: Make QPushButton/QToolButton the same height as QLineEdit

[Yunhe Guo]

guoyunhe added a comment.


  In D25381#564404 , @ndavis wrote:
  
  > If we do make flat toolbuttons buttons (`autoRaise == true`) and non-flat 
buttons the same size, we will need to change the default icon size for all 
buttons to 22px, or we will have a ton of UI regressions. Any non-flat button 
with a fixed icon size is going to have a regression. Anyone who was using 
non-flat buttons with 22px icons to get a bigger clickable area will need to 
switch to 32px icons. This also means that breeze-icons' problem with 32px icon 
style consistency will show itself more often.
  
  
  Thanks for the remind!
  
  By default, QToolButton with 22px icons look good:
  
  F7771214: image.png 
  
  When change to 32px, still okay:
  
  F7771218: image.png 
  
  Maybe this is because Dolphin has all icons in 32px. Do you know any 
application that miss 32px icons in toolbar? Thanks!

REPOSITORY
  R31 Breeze

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

To: guoyunhe, #breeze, #plasma
Cc: ndavis, GB_2, ngraham, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, 
fbampaloukas, ragreen, ZrenBot, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25374: [ResultDelegate] Fix bad text legiblity with certain 3rd party themes

[Filip Fila]

filipf added a comment.


  It should be lighter in the case of dark themes, that's what I get 
empirically and what the code points to. How do I reproduce this?

REPOSITORY
  R112 Milou

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

To: filipf, #plasma, #vdg
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D25374: [ResultDelegate] Fix bad text legiblity with certain 3rd party themes

[Kai Uwe Broulik]

broulik added a comment.


  This makes it quite hard to read on a dark background here
  F7771184: Screenshot_20191119_092956.png 


REPOSITORY
  R112 Milou

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

To: filipf, #plasma, #vdg
Cc: broulik, plasma-devel, LeGast00n, The-Feren-OS-Dev, jraleigh, fbampaloukas, 
GB_2, ragreen, ZrenBot, ngraham, alexeymin, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart