D27958: [SystemTray] Fix item size for very narrow panel

[Nathaniel Graham]

ngraham added a comment.


  After a few days of living with it, I've found that this patch causes the 
following issue with semi-random tray items getting smaller when a pop-up is 
opened:
  
  F8176616: vokoscreenNG-2020-03-14_23-35-16.webm 

  
  Could you investigate?

REPOSITORY
  R120 Plasma Workspace

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

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

D28024: [Baloo KCM] Expose "Index hidden" config setting

[Nathaniel Graham]

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


  All right, let's go for it as-is.

REPOSITORY
  R119 Plasma Desktop

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Tranter Madi]

trmdi updated this revision to Diff 77644.
trmdi added a comment.


  Nate's comment

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27998?vs=77643=77644

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  kcms/workspaceoptions/package/contents/ui/main.qml
  kcms/workspaceoptions/workspaceoptions_kdeglobalssettings.kcfg

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> trmdi wrote in main.qml:204
> If there is a new stuff below this, when you make it visible, the below stuff 
> will jump up.

That already happens with the other labels though. Implementing this behavior 
here would make it inconsistent with the other ones. If we don't want the UI 
jumping around, we should change that in another patch to that this one isn't 
inconsistent with the current state.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Tranter Madi]

trmdi added inline comments.

INLINE COMMENTS

> ngraham wrote in main.qml:204
> I would make this label non-visible when `dndToMoveEnabler.checked` is false, 
> rather than changing its text.

If there is a new stuff below this, when you make it visible, the below stuff 
will jump up.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Nathaniel Graham]

ngraham added a comment.


  Hmm, that's true. How awkward.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Tranter Madi]

trmdi added a comment.


  In D27998#627623 , @ngraham wrote:
  
  > Where? Being wrong in multiple places doesn't make it right. ;)
  
  
  On the left panel.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> main.qml:204
> +
> +QQC2.Label {
> +Layout.fillWidth: true

I would make this label non-visible when `dndToMoveEnabler.checked` is false, 
rather than changing its text.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Tranter Madi]

trmdi updated this revision to Diff 77643.
trmdi added a comment.


  Nate's comment

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27998?vs=77509=77643

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  kcms/workspaceoptions/package/contents/ui/main.qml
  kcms/workspaceoptions/workspaceoptions_kdeglobalssettings.kcfg

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Nathaniel Graham]

ngraham added a comment.


  Where? Being wrong in multiple places doesn't make it right. ;)

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Tranter Madi]

trmdi added a comment.


  In D27998#627605 , @ngraham wrote:
  
  > Right.
  >
  > @bugseforuns's objection is that you used the word "device" to mean 
"partition" in the UI.
  
  
  Both Dolphin and Qt use this word.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D28035: [Applet] Port to ExpandableListItem

[Nathaniel Graham]

ngraham added a comment.


  Yes, that's the last one I need to port. I've already submitted patches to 
port the Printers, Bluetooth, and Networks applets, as they also have the same 
style.

REPOSITORY
  R845 Plasma Vault

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

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

D28024: [Baloo KCM] Expose "Index hidden" config setting

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> ngraham wrote in main.qml:59
> Instead adding these new layouts, you could just set the following on each 
> checkbox that needs to be indented:
> 
>   Layout.leftMargin: !LayoutMirroring.enabled ? Kirigami.Units.largeSpacing : > 0
>   Layout.rightMargin: LayoutMirroring.enabled ? Kirigami.Units.largeSpacing : > 0
> 
> IMO that's a bit cleaner.

The effect would be different, as currently there already is some margin.

Also, this has to be repeated for each item, and I plan to add (at least) one 
more option.

REPOSITORY
  R119 Plasma Desktop

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Nathaniel Graham]

ngraham added a comment.


  Right.
  
  @bugseforuns's objection is that you used the word "device" to mean 
"partition" in the UI.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Tranter Madi]

trmdi added a comment.


  In D27998#627603 , @ngraham wrote:
  
  > What does?
  
  
  dnd from /dev/sda1 -> /dev/sda1 : move
  dnd from /dev/sda1 -> /dev/sda2 : show menu

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Nathaniel Graham]

ngraham added a comment.


  What does?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Tranter Madi]

trmdi added a comment.


  Yes, it means "partition".

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D28024: [Baloo KCM] Expose "Index hidden" config setting

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> main.qml:59
> +
> +ColumnLayout {
> +QQC2.CheckBox {

Instead adding these new layouts, you could just set the following on each 
checkbox that needs to be indented:

  Layout.leftMargin: !LayoutMirroring.enabled ? Kirigami.Units.largeSpacing : 0
  Layout.rightMargin: LayoutMirroring.enabled ? Kirigami.Units.largeSpacing : 0

IMO that's a bit cleaner.

REPOSITORY
  R119 Plasma Desktop

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

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

D28035: [Applet] Port to ExpandableListItem

[Ivan Čukić]

ivan added a comment.


  I like it. Is the device notifier going to look like this as Vaaults tried to 
mimic that one?

REPOSITORY
  R845 Plasma Vault

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

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

D27160: [applets/mediacontroller] Visually refresh media controller plasmoid

[Carson Black]

cblack added inline comments.

INLINE COMMENTS

> kmaterka wrote in ExpandedRepresentation.qml:261
> I have not seen your code yet, I will check tomorrow. Probably because you 
> have something like this:
> 
>   Layout.preferredWidth: parent.width / 2
>   Layout.preferredHeight: parent.height / 2
> 
> Check the last answer from: How to design a multi-level fluid layout in QML 
> .
>  In short words: when possible, do not use Layout.preferredWidth, prefer 
> implicitWidth instead. If you want to have 50-50 ratio between items, set 
> implicitWidth in both items to the same value.

This advice doesn't seem to be applicable here, as the `implicitWidth` of the 
ColumnLayout child isn't able to be mutated due to the fact that all of its 
child items (text) have fixed `implicitWidth`s.

REPOSITORY
  R120 Plasma Workspace

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

To: cblack, #vdg, #plasma, manueljlin, ngraham
Cc: kmaterka, iasensio, ndavis, broulik, gvgeo, davidedmundson, ngraham, 
manueljlin, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27189: Created a top area based on widgets/toparea.svg

[Niccolò Venerandi]

niccolove added a comment.


  I think I have the old network manager. I will submit patches.

REPOSITORY
  R120 Plasma Workspace

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

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

D27189: Created a top area based on widgets/toparea.svg

[Niccolò Venerandi]

niccolove added a comment.


  Weird. I can't reproduce, neither in my compiled nor stable install:
  F8176254: Screenshot_20200314_233208.png 

  But then again, apparently it's not master-y master, so maybe something 
changed?

REPOSITORY
  R120 Plasma Workspace

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

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

D27189: Created a top area based on widgets/toparea.svg

[Nathaniel Graham]

ngraham added a comment.


  Just noticed that there are various system tray applets whose main 
content/toolbar areas need a bit more of a top margin now: F8176236: 
Screenshot_20200314_160647.png  F8176238: 
Screenshot_20200314_160805.png 
  
  It looks like this is not a universal thing and so will need to be fixed 
applet-by-applet. Could you submit patches for those?

REPOSITORY
  R120 Plasma Workspace

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

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

D27160: [applets/mediacontroller] Visually refresh media controller plasmoid

[Konrad Materka]

kmaterka added inline comments.

INLINE COMMENTS

> kmaterka wrote in ExpandedRepresentation.qml:261
> My last comment was not precise. Just check this 
>  for all details :)

As a general rule, you shouldn't refer to parents width in Layouts, including: 
`Layout.maximumWidth: parent.width`. It a (very) common mistake :)

REPOSITORY
  R120 Plasma Workspace

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

To: cblack, #vdg, #plasma, manueljlin, ngraham
Cc: kmaterka, iasensio, ndavis, broulik, gvgeo, davidedmundson, ngraham, 
manueljlin, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27160: [applets/mediacontroller] Visually refresh media controller plasmoid

[Konrad Materka]

kmaterka added inline comments.

INLINE COMMENTS

> kmaterka wrote in ExpandedRepresentation.qml:261
> I have not seen your code yet, I will check tomorrow. Probably because you 
> have something like this:
> 
>   Layout.preferredWidth: parent.width / 2
>   Layout.preferredHeight: parent.height / 2
> 
> Check the last answer from: How to design a multi-level fluid layout in QML 
> .
>  In short words: when possible, do not use Layout.preferredWidth, prefer 
> implicitWidth instead. If you want to have 50-50 ratio between items, set 
> implicitWidth in both items to the same value.

My last comment was not precise. Just check this 
 for all details :)

REPOSITORY
  R120 Plasma Workspace

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

To: cblack, #vdg, #plasma, manueljlin, ngraham
Cc: kmaterka, iasensio, ndavis, broulik, gvgeo, davidedmundson, ngraham, 
manueljlin, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27160: [applets/mediacontroller] Visually refresh media controller plasmoid

[Konrad Materka]

kmaterka added inline comments.

INLINE COMMENTS

> cblack wrote in ExpandedRepresentation.qml:261
> I can't seem to reproduce a binding loop here.

I have not seen your code yet, I will check tomorrow. Probably because you have 
something like this:

  Layout.preferredWidth: parent.width / 2
  Layout.preferredHeight: parent.height / 2

Check the last answer from: How to design a multi-level fluid layout in QML 
.
 In short words: when possible, do not use Layout.preferredWidth, prefer 
implicitWidth instead. If you want to have 50-50 ratio between items, set 
implicitWidth in both items to the same value.

REPOSITORY
  R120 Plasma Workspace

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

To: cblack, #vdg, #plasma, manueljlin, ngraham
Cc: kmaterka, iasensio, ndavis, broulik, gvgeo, davidedmundson, ngraham, 
manueljlin, plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, 
zachus, fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  Generating the toolarea colors from the windows colors and making sure the 
WindowText color still works; is also a solution. But it requires to also 
change the decoration to use this generated color instead of that of a palette, 
and an option to go with it. Also: you would still need an option to disable 
all this, especially when window borders are not "none", or when people want to 
use their palette decoration color.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added a comment.


  For what its worth: what you would need to fix this properly, is a new 
QIcon::Mode for "in active tool area" that you would map the the right colors 
when generating a given pixmap from the svg inside kiconloader. 
  Only then would you be able to cache for a given icon its properly colored 
pixmap for being inside the tool area; in a regular button; active; disabled, 
etc ... without the need to resort to different QPalette as done now (namely 
one palette for active toolarea and one palette for all the rest).

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Hugo Pereira Da Costa]

hpereiradacosta added inline comments.

INLINE COMMENTS

> hpereiradacosta wrote in breezestyle.cpp:4382
> I'll dig in KIconLoader code a bit over the week-end to see what really 
> happens when calling setCustomPalette and resetPalette, to get a sense of how 
> resource consuming this is ...

So,
Digging into the code it seems that this might not be as big of a problem as 
anticipated. 
My fear was that changing the qpalette in kiconloader (setCustomPalette or 
resetPalette) would invalidate the internal cache each time. (which would be 
disastrous, effectively removing all the caching ability). I need to double 
check, but it seems it is not the case: the actual palette is used in the cache 
key. So you end up with a twice larger cache (which is ok) but no cache reset. 
The only thing settingCustomPalette and resetPalette do is change the palette 
key for next cache access. 
So, if confirmed the only remaining thing would be to try avoid too many 
palette creation, by using the palette intrinsic sharing.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27160: [applets/mediacontroller] Visually refresh media controller plasmoid

[Carson Black]

cblack added inline comments.

INLINE COMMENTS

> ngraham wrote in ExpandedRepresentation.qml:261
> there's a massive binding loop somewhere here:
> 
>   
> file:///home/nate/kde/usr/share/plasma/plasmoids/org.kde.plasma.mediacontroller/contents/ui/ExpandedRepresentation.qml:261:21:
>  QML Heading: Binding loop detected for property "height"

I can't seem to reproduce a binding loop here.

REPOSITORY
  R120 Plasma Workspace

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

To: cblack, #vdg, #plasma, manueljlin, ngraham
Cc: iasensio, ndavis, broulik, gvgeo, davidedmundson, ngraham, manueljlin, 
plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, himcesjf, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, ahiemstra, mart

D27996: Fix undefined check in global drawer menu mode

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R169:4d84d3f1d93f: Fix undefined check in global drawer menu 
mode (authored by nicolasfella).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27996?vs=77465=77630

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

AFFECTED FILES
  src/controls/private/globaltoolbar/PageRowGlobalToolBarUI.qml

To: nicolasfella, #kirigami, mart, ngraham
Cc: apol, ngraham, plasma-devel, fbampaloukas, GB_2, domson, dkardarakos, 
ahiemstra, davidedmundson, mart

D27572: [kcm] Drop 'Enable bluetooth integration' setting

[Nicolas Fella]

This revision was automatically updated to reflect the committed changes.
Closed by commit R97:2d92acae05f4: [kcm] Drop Enable bluetooth 
integration setting (authored by nicolasfella).

REPOSITORY
  R97 Bluedevil

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27572?vs=76167=77629

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

AFFECTED FILES
  src/kcmodule/global/global.cpp
  src/kcmodule/global/global.h
  src/kcmodule/global/global.ui

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

D27959: [libtaskmanager] Add ApplicationMenu{ObjectPath, ServiceName} roles to model

[Carson Black]

cblack added a comment.


  Ping.

REPOSITORY
  R120 Plasma Workspace

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

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

D27189: Created a top area based on widgets/toparea.svg

[Niccolò Venerandi]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:f4195688e68e: Created a top area based on 
widgets/toparea.svg (authored by niccolove).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27189?vs=77514=77628

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

AFFECTED FILES
  applets/systemtray/package/contents/ui/ExpandedRepresentation.qml

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

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  In D27669#627465 , @davidre wrote:
  
  > In D27669#627464 , @ndavis wrote:
  >
  > > Do we need to add a Header color set to KColorScheme?
  >
  >
  > How would that help? KIconLoader still wouldn't know which color to use, 
wouldn't it?
  
  
  Maybe I don't understand the internals well enough. My understanding is that 
the titlebar color isn't really part of the colorscheme, it's just read from 
the colorscheme config file. If the titlebar and toolbar were both using the 
Header color set, then they would use the same colors, right?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added a comment.


  In D27669#627464 , @ndavis wrote:
  
  > Do we need to add a Header color set to KColorScheme?
  
  
  How would that help? KIconLoader still wouldn't know which color to use, 
wouldn't it?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Noah Davis]

ndavis added a comment.


  Do we need to add a Header color set to KColorScheme?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[David Redondo]

davidre added a comment.


  In D27669#627444 , @ngraham wrote:
  
  > Why does it have to grab the widget's palette? Can't it simply use the 
colors defined for active/inactive titlebar bg color in the color scheme?
  
  
  Because the iconloader replaces the colors in the stylesheet with the colors 
as defined in the palette.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27998: [KCMs/Workspace] Add option for dndToMove

[Nathaniel Graham]

ngraham added a comment.


  In D27998#627401 , @bugseforuns 
wrote:
  
  > Will popup menu show up if "Move files if on the same device" is selected 
and files are dragged to another partition on the same device?
  >  If so, "Move files if on the same device" label is not accurate.
  
  
  Good point. Maybe it needs to say "Move files if on the same partition or 
device" (yes, "device" is a bit redundant if you're already saying "partition", 
but "device" is a less technical term).

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Why does it have to grab the widget's palette? Can't it simply use the colors 
defined for active/inactive titlebar bg color in the color scheme?

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27669: [kstyle] Tools area

[Carson Black]

cblack added a comment.


  In D27669#627440 , @ngraham wrote:
  
  > Hopefully we can improve KIconLoader to support this use case. If not, we 
can always fall back to using a color for the tools area that's a darkened 
version of the window background color or something, and just set that same 
color for the titlebar with the default breeze color scheme so that they match 
with default settings. That would also presumably better satisfy the people who 
are worried about the possibility of a very colorful Tools Area when using a 
color scheme with a colorful titlebar color, and we wouldn;t have to implement 
code to make the new coloration optional. So maybe we can keep that option in 
our back pocket.
  >
  > JFYI with the latest version, the coloration of non-text items in the 
toolbar is too light when using a color scheme that's pretty close to what 
we're planning to change Breeze to: F8175886: Screenshot_20200314_103911.png 

  
  
  The issue isn't with KIconLoader, the issue is with Qt. There's no API for an 
icon engine to be able to see what widget it's getting an icon for, and thus 
it's unable to grab the widget's palette.

REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D28026: further constrict line parsing of .so files

[Nathaniel Graham]

ngraham added a comment.


  Looks sane to me.

REPOSITORY
  R871 DrKonqi

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

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

D27669: [kstyle] Tools area

[Nathaniel Graham]

ngraham added a comment.


  Hopefully we can improve KIconLoader to support this use case. If not, we can 
always fall back to using a color for the tools area that's a darkened version 
of the window background color or something, and just set that same color for 
the titlebar with the default breeze color scheme so that they match with 
default settings. That would also presumably better satisfy the people who are 
worried about the possibility of a very colorful Tools Area when using a color 
scheme with a colorful titlebar color, and we wouldn;t have to implement code 
to make the new coloration optional. So maybe we can keep that option in our 
back pocket.
  
  JFYI with the latest version, the coloration of non-text items in the toolbar 
is too light when using a color scheme that's pretty close to what we're 
planning to change Breeze to: F8175886: Screenshot_20200314_103911.png 


REPOSITORY
  R31 Breeze

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

To: cblack, #plasma, #breeze, #vdg
Cc: IlyaBizyaev, davidre, davidedmundson, hpereiradacosta, gvgeo, ngraham, 
manueljlin, niccolove, ndavis, plasma-devel, Orage, LeGast00n, 
The-Feren-OS-Dev, cblack, jraleigh, zachus, fbampaloukas, GB_2, ragreen, 
ZrenBot, himcesjf, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, 
apol, ahiemstra, mart

D27199: [Applet]Update layout based on T10470

[George Vogiatzis]

gvgeo added a comment.


  Differently sized icons are about to be restored in D28034 
.
  
  > ...text indicator replaced with a 1px divider...   Is it a good idea to 
tweak it or is it better to leave it like it is?
  
  The alternative is the first version from T10470 
 mocups, right? If #VDG 
 team doesn't like as it is, don't see a 
reason to keep it as it is.
  
  Would be nice to hear about details labels too, which have 0.6 opacity now:
  F8098508: Screenshot_20200211_234124.png 

  
  And of course any other change, to avoid changing the UI with every release.

REPOSITORY
  R116 Plasma Network Management Applet

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

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

D27998: [KCMs/Workspace] Add option for dndToMove

[Patrick Silva]

bugseforuns added a comment.


  Will popup menu show up if "Move files if on the same device" is selected and 
files are dragged to another partition on the same device?
  If so, "Move files if on the same device" label is not accurate.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  add-dndToMove (branched from master)

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

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

D28042: [DrKonqi] Port QRegExp to QRegularExpression

[Ahmad Samir]

ahmadsamir created this revision.
ahmadsamir added reviewers: Plasma, davidedmundson, sitter, apol.
Herald added a project: Plasma.
ahmadsamir requested review of this revision.

REVISION SUMMARY
  Port QRegExp::exactMatch() by using QRegularExpression::anchoredPattern(),
  unless the pattern is already anchored by "^" and "$".
  
  BacktraceLineGdb::parse():
  
  - set QRegularExpression::DotMatchesEverythingOption, since gdb breaks some 
stack frame lines to mulitple lines for readability, e.g.: "#5  
0x7f50e99f776f in QWidget::testAttribute_helper (this=0x6e6440,\n 
attribute=Qt::WA_WState_Created) at kernel/qwidget.cpp:9081\n" this matches 
QRegExp behaviour where a "." char matches new lines by default.
  - Use clustering parentheses (?:) so as not to capture groups that aren't 
needed, less bookkeeping for QRegularExpression/PCRE
  
  Replace QRegularExpression with QString comparison, the latter is usually
  faster.
  
  All unit tests still pass.

REPOSITORY
  R871 DrKonqi

BRANCH
  l-QRE-port (branched from master)

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

AFFECTED FILES
  src/bugzillaintegration/bugzillalib.cpp
  src/gdbhighlighter.cpp
  src/parser/backtraceparser.cpp
  src/parser/backtraceparsergdb.cpp
  src/parser/backtraceparserkdbgwin.cpp

To: ahmadsamir, #plasma, davidedmundson, sitter, apol
Cc: plasma-devel, Orage, LeGast00n, The-Feren-OS-Dev, cblack, jraleigh, zachus, 
fbampaloukas, GB_2, ragreen, ZrenBot, ngraham, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, ahiemstra, mart