D16968: [Folder View] improve label contrast against challenging backgrounds

[Nathaniel Graham]

ngraham updated this revision to Diff 46099.
ngraham added a comment.


  Make the effect a bit more subtle

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16968?vs=45707=46099

BRANCH
  arcpatch-D16968

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

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

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

D16968: [Folder View] improve label contrast against challenging backgrounds

[Nathaniel Graham]

ngraham added a comment.


  That's a great point. In fact, now that I'm looking for it, I see that 
Windows uses a black outline on the white label text for their desktop icons.
  
  Unfortunately, the QML outline effect doesn't seem to look real good, at 
least not with the very small default font size used for the label:
  
  F6438922: Screenshot_20181123_215530.png 

  
  F6438923: Screenshot_20181123_215610.png 

  
  Not sure that'll work.

REPOSITORY
  R119 Plasma Desktop

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

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

D17127: [Device Notifier] Keep the device label & icon after unmounting

[Thomas Surrel]

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

REVISION SUMMARY
  When unmounting a device, the device notifier keeps the information
  of the device and its icon for a few seconds before it disappears.
  But the binding on the label and the icon was making them go blank
  before, leaving an emblem without an icon and a 'This device can
  now be safely removed' notice without the name of the device
  (although it did not always happen, this I am not sure why).

TEST PLAN
  Unmount a removable device with the device notifier: for a few
  seconds, the device name and icon should still be displayed

REPOSITORY
  R120 Plasma Workspace

BRANCH
  arc_notifier_labels (branched from master)

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/FullRepresentation.qml

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

D16170: [Device Notifier] Restore busy indicator

[Thomas Surrel]

thsurrel updated this revision to Diff 46093.
thsurrel added a comment.


  Update the storage size every 5 seconds
  Thank you for catching this one @mart !

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16170?vs=43513=46093

BRANCH
  arc_busyindicator (branched from master)

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

AFFECTED FILES
  applets/devicenotifier/package/contents/ui/DeviceItem.qml
  applets/devicenotifier/package/contents/ui/devicenotifier.qml

To: thsurrel, #plasma, #vdg, broulik, bruns
Cc: mart, cfeck, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D15963: [KonsoleProfiles applet] Fix initial focus

[Thomas Surrel]

This revision was automatically updated to reflect the committed changes.
Closed by commit R114:d6ee79f20adb: [KonsoleProfiles applet] Fix initial focus 
(authored by thsurrel).

REPOSITORY
  R114 Plasma Addons

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D15963?vs=42917=46092

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

AFFECTED FILES
  applets/konsoleprofiles/package/contents/ui/konsoleprofiles.qml

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

D16053: [Window List]Fix initial focus

[Thomas Surrel]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:8ff87951cff8: [Window List]Fix initial focus (authored by 
thsurrel).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16053?vs=43170=46091

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

AFFECTED FILES
  applets/window-list/contents/ui/main.qml

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

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

[Nathaniel Graham]

ngraham added a comment.


  @mart This was all specifically requested by @davidedmundson in D16031 
, unless I misunderstood the request.
  
  @davidedmundson, can you clarify whether or not that is what you wanted?

REPOSITORY
  R120 Plasma Workspace

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

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

D16034: Improve logout screen button appearance and contrast

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> ngraham wrote in Logout.qml:162
> This was discussed endlessly in T9658  and 
> we couldn't find a good way to do it without the result looking really chunky 
> and ugly. I'm open to a batter way to do it if you have some suggestions, 
> though.

I mean T9444 

REPOSITORY
  R120 Plasma Workspace

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

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

D16034: Improve logout screen button appearance and contrast

[Nathaniel Graham]

ngraham added inline comments.

INLINE COMMENTS

> mart wrote in Logout.qml:162
> I would have preferred to not have new icons with a translucent circle 
> behind, but rather doing such circle in QML

This was discussed endlessly in T9658  and 
we couldn't find a good way to do it without the result looking really chunky 
and ugly. I'm open to a batter way to do it if you have some suggestions, 
though.

REPOSITORY
  R120 Plasma Workspace

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

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

D14949: Add option for whether to show the volume change OSD

[Nathaniel Graham]

ngraham added a comment.


  In D14949#364818 , @mart wrote:
  
  > In D14949#312757 , @ngraham 
wrote:
  >
  > > I think the current UI is just fine as the default; no need to change it. 
Simply making it smaller wouldn't really help the submitter's use case very 
much anyway since it would still show up and cover up //something//. However 
I'd be open to making the default appearance configurable, as a small but vocal 
cohort of people don't seem to like it.
  > >
  > > That's material for another patch though.
  >
  >
  > they can do it: they can do a look and feel package which replaces the osd.
  
  
  But that replaces everything else too. Look-and-feel packages aren't a great 
solution to the problem of wanting to change only one or two things, and 
conceptually, they don't scale at all.

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

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

D14514: Install the OSD icons to hicolor

[Nathaniel Graham]

ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  This was already done; the icons were moved to the Plasma theme. I don't 
think this is needed anymore.

REPOSITORY
  R104 KScreen

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

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

D17076: Avoid indeterminism in kcolorschemeeditor

[Olivier Churlaud]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:c9cf4883da4e: Avoid indeterminism in 
kcolorschemeeditors compilation (authored by ochurlaud).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17076?vs=45954=46080

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

AFFECTED FILES
  kcms/colors/CMakeLists.txt

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

D15862: option to specify icons spacing for systray

[Emirald Mateli]

emateli added a comment.


  FWIW I agree with @mart. Just give it a default value and leave it at that. 
More configuration doesn't necessarily mean better.

REPOSITORY
  R120 Plasma Workspace

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

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

D14514: Install the OSD icons to hicolor

[Marco Martin]

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


  (they are also in the plasma theme btw)

REPOSITORY
  R104 KScreen

BRANCH
  master

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

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

D14949: Add option for whether to show the volume change OSD

[Marco Martin]

mart added a comment.


  In D14949#335721 , @graesslin 
wrote:
  
  > It's not about that there are only complaints on Reddit. It's about the 
user group: do we want to deaign so that the small Reddit community is happy or 
do we want to design for the rest? Do we want to make the product more 
complicated (yes that's the result of a config option) to please some people on 
Reddit while at the same time we already have a mechanism to achieve the same?
  
  
  ^^^ this, a thousand times this :D

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

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

D14949: Add option for whether to show the volume change OSD

[Marco Martin]

mart added a comment.


  In D14949#312757 , @ngraham wrote:
  
  > I think the current UI is just fine as the default; no need to change it. 
Simply making it smaller wouldn't really help the submitter's use case very 
much anyway since it would still show up and cover up //something//. However 
I'd be open to making the default appearance configurable, as a small but vocal 
cohort of people don't seem to like it.
  >
  > That's material for another patch though.
  
  
  they can do it: they can do a look and feel package which replaces the osd.

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

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

D15233: Add a tooltip for the appentry in the kicker

[Marco Martin]

mart added a comment.


  In D15233#328690 , @alexeymin 
wrote:
  
  > Why isn't repository for this review set? Which repo should it be?
  >
  > P.S. Guess it is plasma desktop
  >  P.P.S. I edited and set a repo, but still don't see it assigned in diff 
detail...
  
  
  yeah, phab kindof requires the arcanist tool to be used to have things 
correct..

REPOSITORY
  R119 Plasma Desktop

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

To: underwit, #plasma, hein, broulik, davidedmundson
Cc: mart, alexeymin, abetts, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, sebas, apol

D15247: Show tooltips in krunner

[Marco Martin]

mart accepted this revision.
mart added a comment.


  In D15247#341864 , @McPain wrote:
  
  > Additionally, tooltips are glitching if you move the mouse pointer too fast 
between items{F6324386 }
  
  
  there was a recent fix in the kwin morphingpopup effect that should have 
fixed it, i would say go for it

REPOSITORY
  R112 Milou

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

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

D15418: Dim application icon when it is not playing anything

[Marco Martin]

mart added a comment.


  In D15418#331397 , @ngraham wrote:
  
  > Maybe something like this?
  >
  > F6283615: Screenshot_20180924_195355.png 

  
  
  I like this way

REPOSITORY
  R115 Plasma Audio Volume Applet

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

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

D15333: Group Widget + Color Picker = Widget Out Of Bounds

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> McPain wrote in main.qml:111
> > Especially the spacer.width being divided by 2 in the second part of the 
> > statement.
> 
> Look at all the parentheses I wrote ;)
> I divide by two a *subtraction* of spacer.width from width.
> 
> Former we calculated a button size to fit the whole widget vertically (if the 
> layout is horizontal) and vice versa but what if we don't have enough space 
> to fit the widget horizontally?
> 
> We have a 100x100 field. Okay, let height = 90 (margins, etc)
> Then, first button 90x90, spacer 5x90 (still okay), and one more button 90x90 
> (oops, fail: the widget is 185x90)
> 
> Now we check the second dimension and shrink the buttons when necessary.
> First button 45x45, spacer 5x45, second button 45x45. The widget if 95x45, 
> and it fits now.
> 
> So, we have to subtract spacer width from parent width and divide it by 2, 
> because we have two buttons and check if it's smaller than height. If so, we 
> fit in the width.
> 
> Same thing when layout is vertical.

can you put this in the commit description as well?

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

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

D17073: Do not crop albumArt

[Noah Davis]

ndavis added a comment.


  In D17073#364541 , @trmdi wrote:
  
  > In D17073#364393 , @ndavis wrote:
  >
  > > That blur looks nice, but I think the background should be more opaque 
like the Elisa header background.F6437066: Screenshot_20181122_005432.png 

  >
  >
  > Do you want to suggest some numbers ?
  
  
  100%. I think having the same look as Elisa's header would look nice. If you 
think something else looks better go with that.

REPOSITORY
  R119 Plasma Desktop

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

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

D15702: User can define margins for the desktop

[Marco Martin]

mart added a comment.


  I'm against this kind of micro configurations, to me seems the problem could 
have 2 different "proper" solutions, either
  
  - would be needed some protocol between latte dock and plasmashell to 
communicate its panels geometries?
  - Or trying again in having everything in-process of plasmashell.
  
  If i remember correctly the problem was that new functionality was needed in 
PanelView... we could make planelview support.. plugins?

REPOSITORY
  R119 Plasma Desktop

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

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

D15862: option to specify icons spacing for systray

[Marco Martin]

mart added a comment.


  I'm really against this type of micro-options

REPOSITORY
  R120 Plasma Workspace

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

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

D16034: Improve logout screen button appearance and contrast

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> Logout.qml:162
>  id: suspendButton
> -iconSource: "system-suspend"
> +iconSource: "system-suspend-translucent"
>  text: i18nd("plasma_lookandfeel_org.kde.lookandfeel", 
> "Suspend")

I would have preferred to not have new icons with a translucent circle behind, 
but rather doing such circle in QML

REPOSITORY
  R120 Plasma Workspace

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

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

D16170: [Device Notifier] Restore busy indicator

[Marco Martin]

mart added a comment.


  Most of the solidevice engine should not relay on polling..
  after a quick glance at its code: can you test if the free space bar is still 
updating correctly after copying big files from and to?

REPOSITORY
  R120 Plasma Workspace

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

To: thsurrel, #plasma, #vdg, broulik, bruns
Cc: mart, cfeck, ngraham, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol

D16658: Respect the display property of buttons

[Marco Martin]

mart requested changes to this revision.
mart added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> Button.qml:44
> + * and controlRoot.display != AbstractButton.IconOnly == 0 */
> +Kirigami.MnemonicData.label: (controlRoot.display == undefined || 
> controlRoot.display != 0) ? controlRoot.text : ""
>  Shortcut {

controlRoot.hasOwnProperty("display")

> Button.qml:65
> + * and controlRoot.display != AbstractButton.TextOnly == 1 */
> +"icon": controlRoot.icon && (controlRoot.display == undefined || 
> controlRoot.display != 1) ? (controlRoot.icon.name || 
> controlRoot.icon.source) : "",
>  "iconColor": controlRoot.icon && controlRoot.icon.color.a > 0? 
> controlRoot.icon.color : Kirigami.Theme.textColor,

controlRoot.display == undefined needs to become 
controlRoot.hasOwnProperty("display") this would be false with Qt 5.9 and less, 
 protecting controlRoot.display != 1
also add the following comment:
//FIXME: when we can depend from Qt 5.10 do controlRoot.display !== 
T.AbstractButton.TextOnly

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

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

D16658: Respect the display property of buttons

[Marco Martin]

mart added a comment.


  In D16658#359049 , @astippich 
wrote:
  
  > Can I convince you to land this for Kf 5.53 with the promise to clean up 
once frameworks depends on Qt 5.10?
  
  
  it needs at least a final release of Qt 5.12: frameworks need to depend from 
2 Qt releases prior the current one.
  however, i think it's possible to make it work without breaking old releases

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

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

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

D16856: [comic] Sync configuration on write

[Marco Martin]

mart added a comment.


  disk syncs shouldn't be added that lightly, as they are quirte expensive, it 
should always relay on the event compression internal in Applet

REPOSITORY
  R114 Plasma Addons

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

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

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

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> ActionButton.qml:41
>  
> -PlasmaCore.IconItem {
> -id: icon
> -anchors {
> -top: parent.top
> -horizontalCenter: parent.horizontalCenter
> -}
> -width: iconSize
> -height: iconSize
> -
> -colorGroup: PlasmaCore.ColorScope.colorGroup
> -active: mouseArea.containsMouse || root.activeFocus
> +DropShadow {
> +anchors.fill: content

those and the stronger shadows to the clock seems unrelated, as on this 
contrast is doing by darkening the background and not adding unneeded shadows.
this seems more the unrelated change to the login screen to not have the 
blurred background?

REPOSITORY
  R120 Plasma Workspace

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

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

D16435: Also catch new Audio() elements

[Fabian Vogt]

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


  LGTM, but I have no idea about this part of JS.

REPOSITORY
  R856 Plasma Browser Integration

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

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

D17007: Do hash and hex name based output hashes

[Luca Beltrame]

lbeltrame requested changes to this revision.
lbeltrame added a comment.
This revision now requires changes to proceed.


  As a packager: messing up with user configuration without a clear migration 
path is a no-no.

REPOSITORY
  R110 KScreen Library

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

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

D16968: [Folder View] improve label contrast against challenging backgrounds

[Emirald Mateli]

emateli added a comment.


  The classical solution to these problems is always a white text with black 
outline. Works well for both light and dark backgrounds.
  
  Some image from the internet: https://i.imgur.com/pQjoQ.png

REPOSITORY
  R119 Plasma Desktop

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

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

D16968: [Folder View] improve label contrast against challenging backgrounds

[Marco Martin]

mart added a comment.


  tough now it looks a bit too much like a blur around the letters, rather than 
a shadow

REPOSITORY
  R119 Plasma Desktop

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

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

D17107: global menu,rename menuHidden to visible

[Michail Vourlakos]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:5779dde86fe7: global menu,rename menuHidden to visible 
(authored by mvourlakos).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17107?vs=46032=46074

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

AFFECTED FILES
  applets/appmenu/package/contents/ui/main.qml
  applets/appmenu/plugin/appmenumodel.cpp
  applets/appmenu/plugin/appmenumodel.h

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

D16995: Consider identical display models in ControlConfig

[Marco Martin]

mart added a comment.


  does this cause incompatibilities with old configs?

REPOSITORY
  R104 KScreen

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

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

D16996: Add method to write retention to configuration control files

[Marco Martin]

mart accepted this revision.
mart added inline comments.
This revision is now accepted and ready to land.

INLINE COMMENTS

> control.cpp:163
> +}
> +file.write(QJsonDocument::fromVariant(m_info).toJson());
> +//qCDebug(KSCREEN_COMMON) << "Config control saved on: " << 
> file.fileName();

maybe check if it's actually a valid json document?, even just 
!document.isEmpty() after creating the document

REPOSITORY
  R104 KScreen

BRANCH
  0controlWrite

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

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

D17007: Do hash and hex name based output hashes

[Marco Martin]

mart added a comment.


  could a kconfigupdate script be provided to migrate config?
  screwing up multiscreen config between updated is quite of an issue

REPOSITORY
  R110 KScreen Library

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

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

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

[Marco Martin]

mart added a comment.


  In D16988#361739 , @filipf wrote:
  
  > It also conceptually makes more sense to have the search field unfocused 
because searching is not necessarily the main activity done within Kickoff, so 
why have it in focus all the time?
  
  
  is important to have the workflow open the menu + just start typing intact to 
put search at the center of the ui

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

[Marco Martin]

mart requested changes to this revision.
mart added a comment.
This revision now requires changes to proceed.


  this breaks the workflow open menu and just start typing

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

D17095: Fixed comic widget crash

[Pavel Mos]

pavelmos added a comment.


  I have no commit access.

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

To: pavelmos, jriddell, bshah, davidedmundson, #plasma, mart
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17094: Fixed comic widget context menu crash

[Pavel Mos]

pavelmos added a comment.


  I have no commit access.

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

To: pavelmos, jriddell, bshah, davidedmundson, mart
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

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

[Marco Martin]

mart added a comment.


  please add a screenshot for user visible changes

REPOSITORY
  R119 Plasma Desktop

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

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

D17118: drop ancient commented out code

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R161:dd5cfbb6f50e: drop ancient commented out code (authored 
by sitter).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D17118?vs=46067=46072#toc

REPOSITORY
  R161 KActivity Manager Service

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17118?vs=46067=46072

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

AFFECTED FILES
  src/service/Application.cpp

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

D17105: Add a new lookandfeel runner

[Marco Martin]

mart added a comment.


  i wonder if it could make sense putting a piece of the thumbnail image? 
(perhaps would be too squished to make sense)

REPOSITORY
  R114 Plasma Addons

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

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

D17113: Allow disabling KPackage install while building Breeze

[Alexander Schlarb]

ntninja abandoned this revision.
ntninja added a comment.


  Thanks for the feedback, I've looked some more into what the KPackage does 
and I agree that it doesn't make sense to disable it in general. Closing this.

REPOSITORY
  R31 Breeze

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

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

D17113: Allow disabling KPackage install while building Breeze

[Marco Martin]

mart added a comment.


  -1 for disabling random things, we would get reports of people not finding 
the breeze dark look and not knowing why, and if a distro would do this, would 
be even worse

REPOSITORY
  R31 Breeze

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

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

D17112: Allow disabling the build of the file dialog component

[Alexander Schlarb]

ntninja added a comment.


  But what if my application runs sandboxed in the desktop and does its file 
selection through portals? I'd still want it to have maximum desktop 
integration under KDE if possible. Hence I want to include `plasma-integration` 
as well, or is my thinking wrong here?

REPOSITORY
  R135 Integration for Qt applications in Plasma

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

To: ntninja, mlaurent, jriddell, dfaure, davidedmundson
Cc: davidedmundson, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D17119: actually initialize kcrash properly

[Harald Sitter]

This revision was automatically updated to reflect the committed changes.
Closed by commit R161:699dbef8f577: actually initialize kcrash properly 
(authored by sitter).

REPOSITORY
  R161 KActivity Manager Service

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17119?vs=46068=46071

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

AFFECTED FILES
  src/service/Application.cpp

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

D17119: actually initialize kcrash properly

[Harald Sitter]

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

REVISION SUMMARY
  to successfully use kcrash when linking with as-needed (which is a default
  flag on many linux distros) one also needs to call KCrash::initialize.
  
  without this kcrash may not be available for most users.
  
  https://markmail.org/thread/zv5pheijaze72bzs
  
  (kcrash is already in the link target list)

TEST PLAN
  builds; links correctly

REPOSITORY
  R161 KActivity Manager Service

BRANCH
  kcrash

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

AFFECTED FILES
  src/service/Application.cpp

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

D17118: drop ancient commented out code

[Harald Sitter]

sitter updated this revision to Diff 46067.
sitter added a comment.


  more related stuff

REPOSITORY
  R161 KActivity Manager Service

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17118?vs=46066=46067

BRANCH
  no-comment

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

AFFECTED FILES
  src/service/Application.cpp

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

D17118: drop ancient commented out code

[Harald Sitter]

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

REVISION SUMMARY
  it's been disabled since 2013. it stands to reason that we don't need it
  and should we need it in the future I am sure we'll manage to figure out
  how to use kaboutdata...

REPOSITORY
  R161 KActivity Manager Service

BRANCH
  no-comment

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

AFFECTED FILES
  src/service/Application.cpp

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

D16860: touchscreen text controls

[Marco Martin]

This revision was automatically updated to reflect the committed changes.
Closed by commit R858:01411790d480: touchscreen text controls (authored by 
mart).

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D16860?vs=45408=46065

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

AFFECTED FILES
  org.kde.desktop/TextArea.qml
  org.kde.desktop/TextField.qml
  org.kde.desktop/private/MobileCursor.qml
  org.kde.desktop/private/MobileTextActionsToolBar.qml
  org.kde.desktop/private/qmldir

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

D16860: touchscreen text controls

[Marco Martin]

mart added a comment.


  for the future, could maybe be a global eventfilter of the window which does 
the switch of mode when a touch event arrived or when a mouse event 
arrived...will have to be tought trough...

REPOSITORY
  R858 Qt Quick Controls 2: Desktop Style

BRANCH
  phab/textcontrols

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

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

D4929: DrKonqi : lldb and Mac support

[René J . V . Bertin]

rjvbb added inline comments.

INLINE COMMENTS

> davidedmundson wrote in backtracegenerator.cpp:140
> Yes
> 
> Also is it worth adding a "break;" there?

A break instead of or in addition to the return? I think I'd prefer the return, 
unless you think that maybe someday there will be some extra steps to be taken 
after the while loop?

> davidedmundson wrote in lldbrc:7
> Is this AppleTerminal line meant to be here?

Yes, that file is used when the user asks to attach a debugger to the crashed 
process. We shouldn't presume that s/he has Konsole installed, nor that it is 
installed with a wrapper script on the path. So we install a script that starts 
Apple's standard terminal emulator with the proper arguments, and call that.

REPOSITORY
  R871 DrKonqi

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

To: rjvbb, #plasma_workspaces, kfunk, davidedmundson
Cc: plasma-devel, #kde_applications, patrickelectric, kfunk, mart, broulik, 
kde-mac, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D4929: DrKonqi : lldb and Mac support

[David Edmundson]

davidedmundson added inline comments.

INLINE COMMENTS

> rjvbb wrote in backtracegenerator.cpp:140
> You mean move the check on QProcess::Running into the `if 
> (line.startsWith(QLatin1String("Process ")) && line.endsWith(QLatin1String(" 
> detached")))` ?

Yes

Also is it worth adding a "break;" there?

> lldbrc:7
> +[KCrash]
> +Exec=AppleTerminal lldb -p %pid
> +Terminal=true

Is this AppleTerminal line meant to be here?

REPOSITORY
  R871 DrKonqi

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

To: rjvbb, #plasma_workspaces, kfunk, davidedmundson
Cc: plasma-devel, #kde_applications, patrickelectric, kfunk, mart, broulik, 
kde-mac, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol

D17094: Fixed comic widget context menu crash

[Pavel Mos]

pavelmos updated this revision to Diff 46064.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D17094?vs=45994=46064

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

AFFECTED FILES
  applets/comic/comic.cpp
  applets/comic/comic.h

To: pavelmos, jriddell, bshah, davidedmundson
Cc: anthonyfieroni, plasma-devel, ragreen, Pitel, ZrenBot, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart

D4929: DrKonqi : lldb and Mac support

[René J . V . Bertin]

rjvbb added a comment.


  > That'll probably be when we move to Qt6.
  
  Which is something I hope will be as far in the future as possible :)
  
  but
  
  > I doubt it really would make anyone's lives easier.
  
  Moving something outside of what I secretly call the Plasma jealousy universe 
should make life a bit easier for those who now have to argue why they'd want 
to use it. I can (kind of) understand why certain Plasma code would want to use 
the latest Qt5 version and almost why that version would be standardised across 
all Plasma members even though not necessary at all. So there's that too.
  
  > Can you explain why you detach?  Work to just call quit in the script to 
solve the hanging issue?
  
  In a nutshell, detaching before quitting makes the chance of hanging a lot 
smaller and speeds up the quitting too in my experience.
  
  As I said, I can make the variable backend-agnostic if you think that's 
preferable. But maybe it could be a nice junior job or GSoC project to redesign 
the backend so that DrKonqi doesn't have to wait for the debugger to quit. With 
both gdb and lldb it should be possible to obtain a reloaded backtrace from the 
same debugger instance, and that refreh should be a lot faster if you don't 
have to wait for a new debugger instance to start and churn through all loaded 
libraries - and I'm guessing that there will be no hanging issues when you quit 
lldb at DrKonqi's exit.
  And if so, keeping the backend-specific variable where it is could serve as a 
nice little reminder.

INLINE COMMENTS

> davidedmundson wrote in aboutbugreportingdialog.cpp:39
>   icon = QIcon::fromTheme
>   if (icon.isValid()) {
>  setWindowIcon(...)
>   }
> 
> is more standard..
> 
> Though from the docs:
> 
> > Note: On macOS, the window title bar icon is meant for windows representing 
> > documents, and will only show up if a file path is also set.
> 
> We don't set this, is this an issue?

Hmmm, this may have changed after I developed the brunt of this patch (in KDE4 
days). To answer your question, no, it's not a problem.
I'll have to look at this again, to see if this and similar changes still make 
sense.

> davidedmundson wrote in backtracegenerator.cpp:140
> this seems dangerous for the other clients.
> 
> It's not unfeasible for a process to have a load of data still in the buffer 
> when it quits.
> 
> I don't know lldb, but it seems you can probably move this to ~149

You mean move the check on QProcess::Running into the `if 
(line.startsWith(QLatin1String("Process ")) && line.endsWith(QLatin1String(" 
detached")))` ?

REPOSITORY
  R871 DrKonqi

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

To: rjvbb, #plasma_workspaces, kfunk, davidedmundson
Cc: plasma-devel, #kde_applications, patrickelectric, kfunk, mart, broulik, 
kde-mac, ragreen, Pitel, ZrenBot, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol