D25863: Remove traces of old GTK configs

[Aleix Pol Gonzalez]

apol added inline comments.

INLINE COMMENTS

> appearancegtk2.cpp:146
>  {
> -m_settings = QMap {
> -{"toolbar_style", "GTK_TOOLBAR_ICONS"},
> -{"show_icons_buttons", "0"},
> -{"show_icons_menus", "0"},
> -{"primary_button_warps_slider", "false"}
> -};
> +m_settings = QMap {};
>  }

Should be m_settings.clear().

Not a big deal, it does the same, just FYI.

REPOSITORY
  R99 KDE Gtk Configuration Tool

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

To: gikari, ngraham
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

D25863: Remove traces of old GTK configs

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R99:338ce2466e4a: Remove traces of old GTK configs (authored 
by gikari, committed by ngraham).

REPOSITORY
  R99 KDE Gtk Configuration Tool

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25863?vs=71223=71235

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

AFFECTED FILES
  src/appearancegtk2.cpp
  src/appearancegtk3.cpp
  src/gtkconfigkcmodule.cpp

To: gikari, 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

D25580: [System Tray] Unified data model for System Tray items

[Konrad Materka]

kmaterka updated this revision to Diff 71232.
kmaterka added a comment.


  Review fix

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25580?vs=71231=71232

BRANCH
  master

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

AFFECTED FILES
  applets/systemtray/CMakeLists.txt
  applets/systemtray/package/contents/ui/ConfigEntries.qml
  applets/systemtray/package/contents/ui/ConfigGeneral.qml
  applets/systemtray/systemtray.cpp
  applets/systemtray/systemtray.h
  applets/systemtray/systemtraymodel.cpp
  applets/systemtray/systemtraymodel.h

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

D25580: [System Tray] Unified data model for System Tray items

[Konrad Materka]

kmaterka updated this revision to Diff 71231.
kmaterka marked 5 inline comments as done.
kmaterka added a comment.


  Review fix

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25580?vs=70464=71231

BRANCH
  master

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

AFFECTED FILES
  applets/systemtray/CMakeLists.txt
  applets/systemtray/package/contents/ui/ConfigEntries.qml
  applets/systemtray/package/contents/ui/ConfigGeneral.qml
  applets/systemtray/systemtray.cpp
  applets/systemtray/systemtray.h
  applets/systemtray/systemtraymodel.cpp
  applets/systemtray/systemtraymodel.h

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

D25580: [System Tray] Unified data model for System Tray items

[Konrad Materka]

kmaterka marked 5 inline comments as done.
kmaterka added inline comments.

INLINE COMMENTS

> nicolasfella wrote in ConfigEntries.qml:68
> You can try doing
> 
>   for (item of systemTrayModel) {
>   ]
> 
> But I don't know if that will work

Unfortunately model is not iterable. This code will be removed in D22176 
 anyway.

> nicolasfella wrote in systemtraymodel.cpp:60
> roles.insert(ItemType, ...) should be enough

I'm using `enum class` instead of simple `enum`, so explicit cast is required.

> nicolasfella wrote in systemtraymodel.cpp:74
> This could be a candidate for std::find_if

It would be nice, but does QStandardItemModel have an iterator?

> nicolasfella wrote in systemtraymodel.cpp:153
> if (contains) {
> 
>   return stuff
> 
> }
> 
> otherStuff()

OK, changed.

> nicolasfella wrote in systemtraymodel.h:77
> What's the purpose of the *Changed roles?

All of these roles are copied from StatusNotifierItemSource. It might be an 
overkill to support all.
I don't know exactly what was the intention to use "*Changed" roles, there is 
only one comment: `// record what has changed`. I found usages in old KDE4 code:

> ./kde4/kde-workspace/plasma-workspace/applets/systemtray/plugin/protocols/dbussystemtray/dbussystemtraytask.cpp



  void DBusSystemTrayTask::dataUpdated(const QString , const 
Plasma::DataEngine::Data )
  {
  // 
  if (properties["TitleChanged"].toBool() || becomeValid) {
  QString title = properties["Title"].toString();
  // ...
  }
  // ...

REPOSITORY
  R120 Plasma Workspace

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

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

D25860: [plasmoid] Show device battery percentage

[Aleix Pol Gonzalez]

apol added a comment.


  +1 cool! Really missed it.

REPOSITORY
  R97 Bluedevil

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

To: broulik, #plasma, #vdg, drosca
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

D25861: [Notifications] Fix crash accessing ViewTransition attached property

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R120:34ca49210ff4: [Notifications] Fix crash accessing 
ViewTransition attached property (authored by broulik).

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25861?vs=71219=71225

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

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

To: broulik, #plasma, ngraham, davidedmundson
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

D25861: [Notifications] Fix crash accessing ViewTransition attached property

[Nathaniel Graham]

ngraham added a comment.


  No more crashes so far. :)

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, ngraham, davidedmundson
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

D25863: Remove traces of old GTK configs

[Mikhail Zolotukhin]

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

REVISION SUMMARY
  Since slider behavior is controlled by kded daemon, gtk kcm has no control 
over it now.
  
  It appears, that some traces of the warps-slider config were not removed. Now 
it is removed along with some other ones redundant lines.

TEST PLAN
  Check that GTK KCM no longer crash, if entered from System Settings.

REPOSITORY
  R99 KDE Gtk Configuration Tool

BRANCH
  remove-traces-of-warps-slider (branched from master)

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

AFFECTED FILES
  src/appearancegtk2.cpp
  src/appearancegtk3.cpp
  src/gtkconfigkcmodule.cpp

To: gikari
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

D24070: Don't use toolTipMainText to show info, rather use the second line

[Matej Mrenica]

mthw updated this revision to Diff 71222.
mthw added a comment.


  Tried to implement your suggestion, hopefully correctly, works correctly so 
far.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=71175=71222

BRANCH
  master

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

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

To: mthw, ngraham, #vdg, #plasma, broulik, ndavis
Cc: ndavis, 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

D25861: [Notifications] Fix crash accessing ViewTransition attached property

[Kai Uwe Broulik]

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


  - remove leftover debugging

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25861?vs=71217=71219

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

AFFECTED FILES
  applets/notifications/package/contents/ui/FullRepresentation.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

D25861: [Notifications] Fix crash accessing ViewTransition attached property

[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
  There's something really funky going on in this...
  Avoid using `ViewTransition` attached property be just setting the animation 
target before triggering the model removal.
  
  BUG: 414066

TEST PLAN
  - Remains to be seen whether the crash is gone but gdb showed it was related 
to `QQuickViewTransitionAttached` which we no longer use
  - Hasn't crashed for a while lol

REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  applets/notifications/package/contents/ui/FullRepresentation.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

D25860: [plasmoid] Show device battery percentage

[Kai Uwe Broulik]

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

REVISION SUMMARY
  While it is typically shown in Battery Monitor, it can still be useful to 
have it in Bluetooth plasmoid.

TEST PLAN
  My headphones also support it but only with newer Bluez upower versions
  F7808306: Screenshot_20191210_182515.png 


REPOSITORY
  R97 Bluedevil

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

AFFECTED FILES
  src/applet/package/contents/ui/DeviceItem.qml

To: broulik, #plasma, #vdg, 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

D25851: [Notifications] Add quick reply feature

[Nathaniel Graham]

ngraham added a comment.


  All right, let's stay with a single line for now.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, #vdg
Cc: 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

D25851: [Notifications] Add quick reply feature

[Kai Uwe Broulik]

broulik added a comment.


  I'm not sure. `TextArea` is a lot more annoying to work with, and the replies 
typically are short. I think Android also only has single line.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, #vdg
Cc: 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

D24070: Don't use toolTipMainText to show info, rather use the second line

[Kai Uwe Broulik]

broulik added a comment.


  You can just do something like
  
var parts = [];
if (powerManagementDisabled) {
parts.push(i18n("Powermanagement disabled"));
}
if (pmSource.data["AC Adapter"] && pmSource.data["AC Adapter"]["Plugged 
in"]) {
parts.push(blabla);
} else {
parts.push(blablabla);
}
parts.push(whatever other texts);
return parts.join("\n");

REPOSITORY
  R120 Plasma Workspace

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

To: mthw, ngraham, #vdg, #plasma, broulik, ndavis
Cc: ndavis, 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

D25846: [Cuttlefish] Use the same actions in all menus

[Nathaniel Graham]

ngraham added a comment.


  Nice, a very good refactor.

INLINE COMMENTS

> Menu.qml:24
>  
> -import QtQuick 2.5
> -import Qt.labs.platform 1.0
> +import QtQuick 2.3
> +import QtQuick.Controls 2.5 as QQC2

unrelated

> Menu.qml:47
> +property Component submenuComponent: actionsMenu
> +//renamed to work on both Qt 5.9 and 5.10
> +property Component itemDelegate: QQC2.MenuItem { }

Do we need to care about those? Plasma's minimum Qt version is 5.12 right now.

REPOSITORY
  R118 Plasma SDK

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

To: davidre, #plasma, ngraham, cblack
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

D25851: [Notifications] Add quick reply feature

[Nathaniel Graham]

ngraham added a comment.


  Beautiful! Can we make it handle multi-line text input by becoming taller?

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma, #vdg
Cc: 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

D25851: [Notifications] Add quick reply feature

[Kai Uwe Broulik]

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

REVISION SUMMARY
  This adds a quick reply feature with a text field inline in the notification 
popup.
  An action named `reply` will spawn the text field and a `NotificationReplied` 
signal is emitted then.
  There's additional kde hints for changing the placeholder text (defaults to 
"Type a reply..."), submit button text (defaults to "Send") and submit button 
icon name (defaults to "document-send", that paper aeroplane icon).

TEST PLAN
  Needs D25832  for KWin
  Doesn't work on Wayland because there notifications never ever ever get focus 
and I am sick and tired of running after KWin people to have this addressed.
  
  Notification, just has a normal button - should we have some form of 
indication that it will spawn a quick reply here actually instead of e.g. 
bringing up the app or something?
  F7807886: Screenshot_20191210_19.png 

  Default placeholder and button
  F7807887: Screenshot_20191210_155720.png 

  Custom placeholder and button
  F7807890: Screenshot_20191210_155642.png 

  Button enabled after typing (can also hit Return/Enter so send)
  F7807891: Screenshot_20191210_155652.png 


REPOSITORY
  R120 Plasma Workspace

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

AFFECTED FILES
  applets/notifications/notificationapplet.cpp
  applets/notifications/notificationapplet.h
  applets/notifications/package/contents/ui/NotificationItem.qml
  applets/notifications/package/contents/ui/NotificationPopup.qml
  applets/notifications/package/contents/ui/NotificationReplyField.qml
  applets/notifications/package/contents/ui/global/Globals.qml
  libnotificationmanager/dbus/org.freedesktop.Notifications.xml
  libnotificationmanager/notification.cpp
  libnotificationmanager/notification.h
  libnotificationmanager/notification_p.h
  libnotificationmanager/notifications.cpp
  libnotificationmanager/notifications.h
  libnotificationmanager/notificationsmodel.cpp
  libnotificationmanager/notificationsmodel.h
  libnotificationmanager/server.cpp
  libnotificationmanager/server.h
  libnotificationmanager/server_p.cpp
  libnotificationmanager/server_p.h

To: broulik, #plasma, #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

D25428: Improved support for desktop "Folder View"

[Nathaniel Graham]

ngraham added a comment.


  Does `grabChanged` work?

REPOSITORY
  R119 Plasma Desktop

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

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

D25428: Improved support for desktop "Folder View"

[Nathaniel Graham]

ngraham added a comment.


  In D25428#574844 , @steffenh wrote:
  
  > and second we need to change QtQuick from 2.8 to 2.12.
  
  
  That's fine.

REPOSITORY
  R119 Plasma Desktop

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

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

D25428: Improved support for desktop "Folder View"

[Steffen Hartleib]

steffenh added a comment.


  Ok a quick look at TapHandler, first I am missing a positionChanged signal in 
the TapHandler and second we need to change QtQuick from 2.8 to 2.12.

REPOSITORY
  R119 Plasma Desktop

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

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

D25844: Don't compress tasks before grouping, when task manager is vertical.

[George Vogiatzis]

gvgeo updated this revision to Diff 71191.
gvgeo retitled this revision from "Add grouping options in Icons-only Task 
Manager" to "Don't compress tasks before grouping, when task manager is 
vertical.".
gvgeo edited the summary of this revision.
gvgeo added a comment.


  In D25844#574741 , @GB_2 wrote:
  
  > I already made a patch for this: D23242 
  
  
  
  
  In D25844#574749 , @ngraham wrote:
  
  > @gvgeo since we already have a patch that disables grouping, maybe you 
could use this one to implement only the change to the compressing animation?
  
  
  My mistake, should have checked here too first.
  I left only the compressing part for the default task manager.

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25844?vs=71182=71191

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

AFFECTED FILES
  applets/taskmanager/package/contents/ui/code/layout.js

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

D25843: Restore notifier on AnimationDurationFactor that was lost during KConfig XT port

[Kevin Ottens]

ervin added a comment.


  In D25843#574736 , @davidedmundson 
wrote:
  
  > It's true that's the only user of it right now, so this is technically 
correct.
  >
  > I don't think it's a problem for us to emit changes for the other things. 
KConfigWatcher allows a user to see which things got changed as fine grain 
filtering in the receiver, there's basically no downside.
  
  
  Well, that's still more traffic on the bus for potentially nothing (more 
processing, more processes waking up). It might be negligible here but I'd be 
concerned at having this for all settings all the time.

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, davidedmundson
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

D25846: [Cuttlefish] Use the same actions in all menus

[David Redondo]

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

REVISION SUMMARY
  Defines the actions in one place and uses them for the context menus and
  buttons in the two preview panes. This reduces code duplication, adds
  actions to the context menu which couldn't be triggered from there before
  and makes it easier to add new features with only needing to touch one place
  instead of potentially three. Furthermore this also reduces the risk of bugs
  caused by copy-pasting, for example the screenshot actions didn't work before
  in responsive mode.
  The actionsMenu component is based on the one from Kirigami with the same 
name.

TEST PLAN
  Everything works

REPOSITORY
  R118 Plasma SDK

BRANCH
  actions (branched from master)

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

AFFECTED FILES
  cuttlefish/package/contents/ui/Actions.qml
  cuttlefish/package/contents/ui/Menu.qml
  cuttlefish/package/contents/ui/Preview.qml
  cuttlefish/package/contents/ui/ResponsivePreview.qml
  cuttlefish/package/contents/ui/cuttlefish.qml

To: davidre, #plasma, ngraham, cblack
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

D25841: [Cuttlefish] Set context properties before loading file

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R118:e56bc283019f: [Cuttlefish] Set context properties before 
loading file (authored by davidre).

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25841?vs=71180=71192

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

AFFECTED FILES
  cuttlefish/src/main.cpp

To: davidre, #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

D25844: Add grouping options in Icons-only Task Manager

[Nathaniel Graham]

ngraham added a comment.


  In D25844#574762 , @davidedmundson 
wrote:
  
  > To give history
  >
  > We had a UX in Plasma 4 where choosing between an analog and digital clock 
would be an option in the main clock applet, but choosing the binary clock 
would be a completely different interface - and that doesn't make sense. 
  >  Also we want to reduce the size of our config UIs without losing features.
  
  
  Thanks David! So we moved in the direction of separating the presentation 
rather than grouping it. I almost wonder if it would make more sense to do the 
opposite thing group everything into a single applet wherever possible, but 
allow the presentation to be swapped at will--perhaps even with 
GHNS-downloadable themes or something. So there would be one clock applet, but 
it could look like an analog clock, a digital clock, a binary clock, or a fuzzy 
clock, and you could download a Mickey Mouse clock face, a Star Wars clock 
face, and so on.

REPOSITORY
  R119 Plasma Desktop

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

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

D25844: Add grouping options in Icons-only Task Manager

[David Edmundson]

davidedmundson added a comment.


  > I'm not sure of the history behind why this split is presented in the form 
of two different applets
  
  To give history
  
  We had a UX in Plasma 4 where choosing between an analog and digital clock 
would be an option in the main clock applet, but choosing the binary clock 
would be a completely different interface - and that doesn't make sense. 
  Also we want to reduce the size of our config UIs without losing features.

REPOSITORY
  R119 Plasma Desktop

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

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

D23242: Add grouping and "Show Only Minimized" options to icons-only task manager

[Nathaniel Graham]

ngraham added a comment.


  FWIW the bug this fixes (https://bugs.kde.org/show_bug.cgi?id=368430) is 
starting to accumulate duplicates. There seems to be some user demand for this.

REPOSITORY
  R119 Plasma Desktop

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

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

D25844: Add grouping options in Icons-only Task Manager

[Nathaniel Graham]

ngraham added a comment.


  @gvgeo since we already have a patch that disables grouping, maybe you could 
use this one to implement only the change to the compressing animation?

REPOSITORY
  R119 Plasma Desktop

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

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

D25740: Add item spacing to size hint

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R31:74285aea999c: Add item spacing to size hint (authored by 
broulik).

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25740?vs=70903=71189

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: broulik, ndavis, hpereiradacosta
Cc: 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

D25844: Add grouping options in Icons-only Task Manager

[Björn Feber]

GB_2 added a comment.


  I already made a patch for this: D23242 

REPOSITORY
  R119 Plasma Desktop

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

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

D25844: Add grouping options in Icons-only Task Manager

[Nathaniel Graham]

ngraham added a comment.


  In D25844#574728 , @gvgeo wrote:
  
  > In D25844#574714 , @broulik 
wrote:
  >
  > > So you want to be able to not group apps in IOTM, that is literally what 
IOTM is about, isnt it?
  >
  >
  > I don't know what the reasoning between splitting the task managers is.
  >  But from what I understand, Icons-Only means no text.
  >  And sorting is also removed.
  
  
  Icons-Only implements the "Dock" paradigm and organizes things on the basis 
of apps, not windows. So the name doesn't make it clear, but the Icons-Only 
Task Manager is much more than just "the regular task manager without labels".
  
  I'm not sure of the history behind why this split is presented in the form of 
two different applets rather than a single one with configurable options. In 
the backend, they're both using the same code so this is easily possible. 
Personally I would like for there to be a single task manager that exposes 
three different modes in the config window:
  
  - Legacy Windows Vista and before mode (current default)
  - ^^, but without labels (Windows 10 style with grouping mode turned off)
  - macOS style Dock mode (current Icons-Only Task Manager)
  
  This would call for some serious discussion before implementing it though.

REPOSITORY
  R119 Plasma Desktop

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

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

D25843: Restore notifier on AnimationDurationFactor that was lost during KConfig XT port

[David Edmundson]

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


  It's true that's the only user of it right now, so this is technically 
correct.
  
  I don't think it's a problem for us to emit changes for the other things. 
KConfigWatcher allows a user to see which things got changed as fine grain 
filtering in the receiver, there's basically no downside.
  As a general rule I'm trying to move more of Plasma to use this.
  
  Personally I wouldn't bother changing this, as we'll only have to change it 
back.

REPOSITORY
  R119 Plasma Desktop

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

To: crossi, #plasma, ervin, davidedmundson
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

D25844: Add grouping options in Icons-only Task Manager

[George Vogiatzis]

gvgeo added inline comments.

INLINE COMMENTS

> layout.js:88
> +//  Fit more tasks in this case, that is possible to cut text, 
> before combining tasks.
> +return Math.ceil(length / maximum) * maxStripes() + 1;
> +}

This has an effect mostly for small displays. On bigger displays this has a 
small effect.
If this is not by design could be instead:

  Math.Ceil(Math.floor(length / maximum) * maxStripes() * 1.25);

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #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

D25845: UpdatesPage: Fix references to \x9C in comments

[Alexander Potashev]

This revision was automatically updated to reflect the committed changes.
Closed by commit R134:d25579fe4236: UpdatesPage: Fix references to \x9C in 
comments (authored by aspotashev).

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25845?vs=71187=71188

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

AFFECTED FILES
  discover/qml/UpdatesPage.qml

To: aspotashev, ngraham
Cc: 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

D25845: UpdatesPage: Fix references to \x9C in comments

[Nathaniel Graham]

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


  Thanks!

REPOSITORY
  R134 Discover Software Store

BRANCH
  9c

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

To: aspotashev, ngraham
Cc: 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

D25844: Add grouping options in Icons-only Task Manager

[George Vogiatzis]

gvgeo added a comment.


  In D25844#574714 , @broulik wrote:
  
  > So you want to be able to not group apps in IOTM, that is literally what 
IOTM is about, isnt it?
  
  
  I don't know what the reasoning between splitting the task managers is.
  But from what I understand, Icons-Only means no text.
  And sorting is also removed.

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #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

D25845: UpdatesPage: Fix references to \x9C in comments

[Alexander Potashev]

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

TEST PLAN
  none

REPOSITORY
  R134 Discover Software Store

BRANCH
  9c

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

AFFECTED FILES
  discover/qml/UpdatesPage.qml

To: aspotashev
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

D25840: [Icons KCM] Use help-browser icon

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:0381ed06a07a: [Icons KCM] Use help-browser icon (authored 
by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25840?vs=71177=71186

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

AFFECTED FILES
  kcms/icons/main.cpp

To: broulik, #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

D25844: Add grouping options in Icons-only Task Manager

[Kai Uwe Broulik]

broulik added a comment.


  So you want to be able to not group apps in IOTM, that is literally what IOTM 
is about, isnt it?

REPOSITORY
  R119 Plasma Desktop

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

To: gvgeo, #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

D25844: Add grouping options in Icons-only Task Manager

[George Vogiatzis]

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

REVISION SUMMARY
  Expose grouping options from 'Task Manager' to 'Icons-only Task Manager'.
  Also disable the behavior of compressing the tasks before grouping them 
together,
  when taskbar is vertical or Icons-only, for a more uniform look and behavior.
  Bug: 368430
  Bug: 408437

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  applets/taskmanager/package/contents/ui/ConfigBehavior.qml
  applets/taskmanager/package/contents/ui/code/layout.js
  applets/taskmanager/package/contents/ui/main.qml

To: gvgeo, #plasma, #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

D25842: [Cuttlefish] Replace deprectated method KDeclarative::setupBindings()

[David Redondo]

This revision was automatically updated to reflect the committed changes.
Closed by commit R118:43c4d215bd5e: [Cuttlefish] Replace deprectated method 
KDeclarative::setupBindings() (authored by davidre).

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25842?vs=71181=71184

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

AFFECTED FILES
  cuttlefish/src/main.cpp

To: davidre, #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

D25792: Add notifiers to workspace options kcfg

[Cyril Rossi]

crossi added a comment.


  Thanks  @davidedmundson for spotting this regression.
  
  Actually,  only AnimationDurationFactor should notify.
  
  From D25586 
  See kcms/workspaceoptions/workspaceoptions.cpp line 221.

REPOSITORY
  R119 Plasma Desktop

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

To: davidedmundson, ervin
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

D25843: Restore notifier on AnimationDurationFactor that was lost during KConfig XT port

[Cyril Rossi]

crossi created this revision.
crossi added reviewers: Plasma, ervin, davidedmundson.
Herald added a project: Plasma.
Herald added a subscriber: plasma-devel.
crossi requested review of this revision.

REVISION SUMMARY
  Following D25792  and D25586 
 only AnimationDurationFactor notifies

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/workspaceoptions/workspaceoptions_kdeglobalssettings.kcfgc

To: crossi, #plasma, ervin, davidedmundson
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

D25842: [Cuttlefish] Replace deprectated method KDeclarative::setupBindings()

[David Redondo]

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

REVISION SUMMARY
  Replace method call like described in the documentation. Also we don't need to
  cast  because a QQmlApplicationEngine is a QQmlEngine.

TEST PLAN
  compiles

REPOSITORY
  R118 Plasma SDK

BRANCH
  deprectaed (branched from master)

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

AFFECTED FILES
  cuttlefish/src/main.cpp

To: davidre, #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

D25841: [Cuttlefish] Set context properties before loading file

[David Redondo]

davidre updated this revision to Diff 71180.
davidre added a comment.


  codestyle++

REPOSITORY
  R118 Plasma SDK

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25841?vs=71179=71180

BRANCH
  contextprops (branched from master)

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

AFFECTED FILES
  cuttlefish/src/main.cpp

To: davidre, #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

D25841: [Cuttlefish] Set context properties before loading file

[David Redondo]

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

REVISION SUMMARY
  Otherwise we load the file and the context properties are still undefined
  leading to a buch ReferenceErrors because they are undefined.

TEST PLAN
  Still works - less warnings

REPOSITORY
  R118 Plasma SDK

BRANCH
  contextprops (branched from master)

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

AFFECTED FILES
  cuttlefish/src/main.cpp

To: davidre, #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

D25838: [Applets/Kickoff] Show apps by their real names on top by default

[Noah Davis]

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


  I agree with this.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  show-apps-by-name (branched from master)

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

To: ngraham, #vdg, #plasma, ndavis
Cc: ndavis, michaeltunnell, 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

D25428: Improved support for desktop "Folder View"

[Marco Martin]

mart added inline comments.

INLINE COMMENTS

> FolderItemDelegate.qml:213
> +
> +MouseArea {
> +id: mousearea

How much of this can be done with pointer handlers?
https://doc.qt.io/qt-5/qml-qtquick-taphandler.html
quickly looking at this mousearea it seems that a taphandler coould be enough.
Would be good if somehow an extra mousearea can be avoided

REPOSITORY
  R119 Plasma Desktop

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

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

D25840: [Icons KCM] Use help-browser icon

[Kai Uwe Broulik]

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

REVISION SUMMARY
  `system-help` appears to be an action icon and not available in larger size 
in Adwaita, add `help-browser` before that to look nicer.

TEST PLAN
  5.17
  
  Before (note the tiny green help box in Adwaita preview)
  F7807487: Screenshot_20191210_123352.png 

  After
  F7807488: Screenshot_20191210_123409.png 

  Oxgyen now also matches the "lifebelt" paradigm :)

REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/icons/main.cpp

To: broulik, #plasma, #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

D25837: [Style KCM] Fit widget style preview better

[Kai Uwe Broulik]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:4b444eb2031a: [Style KCM] Fit widget style preview better 
(authored by broulik).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25837?vs=71170=71176

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

AFFECTED FILES
  kcms/style/package/contents/ui/main.qml

To: broulik, #plasma, #vdg, ngraham
Cc: 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

D25837: [Style KCM] Fit widget style preview better

[Nathaniel Graham]

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


  Nice!

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg, ngraham
Cc: 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

D24070: Don't use toolTipMainText to show info, rather use the second line

[Matej Mrenica]

mthw updated this revision to Diff 71175.
mthw added a comment.


  The second line is everywhere now. I guessed it would make more sense. But if 
you dont't like it that way, I can remove it.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D24070?vs=69910=71175

BRANCH
  master

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

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

To: mthw, ngraham, #vdg, #plasma, broulik, ndavis
Cc: ndavis, 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

D25835: [kcms/launchfeedback] Port to KConfig notifiers

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:b9a54826350c: [kcms/launchfeedback] Port to KConfig 
notifiers (authored by davidedmundson).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25835?vs=71161=71172

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

AFFECTED FILES
  kcms/launch/launchfeedback.cpp
  kcms/launch/launchfeedback.h
  kcms/launch/launchfeedbacksettings.kcfgc

To: davidedmundson, zzag
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

D25837: [Style KCM] Fit widget style preview better

[Kai Uwe Broulik]

broulik updated this revision to Diff 71170.
broulik retitled this revision from "[Style KCM] Horizontally center widget 
style preview" to "[Style KCM] Fit widget style preview better".
broulik edited the summary of this revision.
broulik edited the test plan for this revision.
broulik added a comment.


  - Scale down if necessary

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25837?vs=71167=71170

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

AFFECTED FILES
  kcms/style/package/contents/ui/main.qml

To: broulik, #plasma, #vdg
Cc: 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

D25837: [Style KCM] Horizontally center widget style preview

[Kai Uwe Broulik]

broulik added a comment.


  In D25837#574591 , @ngraham wrote:
  
  > Could we scale the previews within the delegates down a bit instead?
  
  
  Oooh, good idea. I bet I could just use `scale` and everything should 
magically work. Not sure how blurry it will get, though, but definitely worth a 
try.

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg
Cc: 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

D25826: [emojier] disable session manager

[Nathaniel Graham]

ngraham accepted this revision.
ngraham added a comment.


  I was thinking of this thing as an app, since that's how we present it. But I 
guess if you don't think of it as an app, that makes sense.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

To: davidedmundson, #plasma, apol, ngraham
Cc: 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

D25837: [Style KCM] Horizontally center widget style preview

[Nathaniel Graham]

ngraham added a comment.


  Could we scale the previews within the delegates down a bit instead?

REPOSITORY
  R119 Plasma Desktop

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

To: broulik, #plasma, #vdg
Cc: 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

D25838: [Applets/Kickoff] Show apps by their real names on top by default

[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 Kickoff shows apps' captions/subtitles on top, with the real name 
below them.
  This is semantically backwards and is also inconsistent with the order with 
which app
  names and captions/subtitles are presented in most other places (Task Manager 
tooltips,
  other launchers, other desktop environments, other operating systems).
  
  This patch reverses the default ordering to put app names on top and fix the 
above issues.
  
  IMO it would even make sense to remove this option and its code entirely, and 
we could
  still do that, but I just wanted to flip the defulat for now to avoid undue 
controversy.

TEST PLAN
  App names are now on top by default with captions/subtitles below them

REPOSITORY
  R119 Plasma Desktop

BRANCH
  show-apps-by-name (branched from master)

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

AFFECTED FILES
  applets/kickoff/package/contents/config/main.xml

To: ngraham, #vdg, #plasma
Cc: michaeltunnell, 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

D25837: [Style KCM] Horizontally center widget style preview

[Kai Uwe Broulik]

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

REVISION SUMMARY
  In case the widget style is too wide. The delegates are already quite large 
as they is so I'd prefer not having them any bigger (or dynamically adjust), or 
else I will only be able to fix a single column of previews now.

TEST PLAN
  Before
  F7807399: Screenshot_20191210_113044.png 

  After
  F7807400: Screenshot_20191210_113020.png 


REPOSITORY
  R119 Plasma Desktop

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

AFFECTED FILES
  kcms/style/package/contents/ui/main.qml

To: broulik, #plasma, #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

D25711: Remove scrollbar behavior option in GTK KCM

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R99:abffd8d7c5d8: Remove scrollbar behavior option in GTK KCM 
(authored by gikari, committed by ngraham).

REPOSITORY
  R99 KDE Gtk Configuration Tool

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25711?vs=70810=71166

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

AFFECTED FILES
  src/abstractappearance.cpp
  src/abstractappearance.h
  src/appearancegtk2.cpp
  src/appearancegtk3.cpp
  src/appearencegtk.cpp
  src/gtkconfigkcmodule.cpp
  src/ui/gui.ui
  tests/configsavetest.cpp

To: gikari, #vdg, ngraham
Cc: 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

D25703: Sync scrollbar behavior with GTK applications

[Nathaniel Graham]

This revision was automatically updated to reflect the committed changes.
Closed by commit R99:68787ff9d3d4: Sync scrollbar behavior with GTK 
applications (authored by gikari, committed by ngraham).

REPOSITORY
  R99 KDE Gtk Configuration Tool

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25703?vs=70796=71165

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

AFFECTED FILES
  kded/configeditor.cpp
  kded/configvalueprovider.cpp
  kded/configvalueprovider.h
  kded/gtkconfig.cpp
  kded/gtkconfig.h

To: gikari, cblack, #vdg, ngraham, apol
Cc: 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

D25836: Remove redundant finder

[Aleix Pol Gonzalez]

This revision was automatically updated to reflect the committed changes.
Closed by commit R99:0b815d1a88d5: Remove redundant finder (authored by apol).

REPOSITORY
  R99 KDE Gtk Configuration Tool

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25836?vs=71163=71164

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

AFFECTED FILES
  cmake/modules/FindGTK2.cmake

To: apol, ngraham
Cc: 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

D25836: Remove redundant finder

[Nathaniel Graham]

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


  Thanks!

REPOSITORY
  R99 KDE Gtk Configuration Tool

BRANCH
  master

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

To: apol, ngraham
Cc: 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

D25836: Remove redundant finder

[Aleix Pol Gonzalez]

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

REVISION SUMMARY
  It's already available in upstream cmake

TEST PLAN
  built kde-gtk-config

REPOSITORY
  R99 KDE Gtk Configuration Tool

BRANCH
  master

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

AFFECTED FILES
  cmake/modules/FindGTK2.cmake

To: 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

D24070: Don't use toolTipMainText to show info, rather use the second line

[Kai Uwe Broulik]

broulik added a comment.


  Probably:
  
  > MainText: Battery at X%
  >  SubText: Powermanagement disabled\n
  >  HH:MM left

REPOSITORY
  R120 Plasma Workspace

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

To: mthw, ngraham, #vdg, #plasma, broulik, ndavis
Cc: ndavis, 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

D24070: Don't use toolTipMainText to show info, rather use the second line

[Matej Mrenica]

mthw added inline comments.

INLINE COMMENTS

> broulik wrote in batterymonitor.qml:56
> You still haven't addressed this. Don't just `return` when 
> `powermanagementDisabled`, I would still want to see my battery state when 
> this is the case?

OK, would you like to see:
MainText: Power management disabled
SubText: Battery at X%, HH:MM left
Or:
MainText: Battery at X% 
SubText:  HH:MM left
Or something else?

REPOSITORY
  R120 Plasma Workspace

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

To: mthw, ngraham, #vdg, #plasma, broulik, ndavis
Cc: ndavis, 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

D25826: [emojier] disable session manager

[Aleix Pol Gonzalez]

apol added a comment.


  In D25826#574375 , @ngraham wrote:
  
  > Hmm, I want this thing to be restored if the window was open when I 
restarted the machine though, right?
  
  
  Why? In case you forgot to choose an emoji?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

To: davidedmundson, #plasma, apol
Cc: 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

D25835: [kcms/launchfeedback] Port to KConfig notifiers

[David Edmundson]

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

REVISION SUMMARY
  This gets rid of all C++ code in this KCM!
  
  It gets rid of a kwin tie in where one shouldn't exist and also exposes
  a notification that the taskmanager should be able to hook into -
  something currently missing as it loads this setting.

TEST PLAN
  Changed setting, kwin updates automatically

REPOSITORY
  R119 Plasma Desktop

BRANCH
  master

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

AFFECTED FILES
  kcms/launch/launchfeedback.cpp
  kcms/launch/launchfeedback.h
  kcms/launch/launchfeedbacksettings.kcfgc

To: davidedmundson
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

D24070: Don't use toolTipMainText to show info, rather use the second line

[Kai Uwe Broulik]

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

INLINE COMMENTS

> broulik wrote in batterymonitor.qml:56
> You can probably just return `plasmoid.title` in the default case.
> Also, what's the reason for not showing percentage when power management is 
> disabled? This isn't about the service not running or being a desktop PC, 
> this is true when screens are forced on.

You still haven't addressed this. Don't just `return` when 
`powermanagementDisabled`, I would still want to see my battery state when this 
is the case?

REPOSITORY
  R120 Plasma Workspace

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

To: mthw, ngraham, #vdg, #plasma, broulik, ndavis
Cc: ndavis, 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

D24070: Don't use toolTipMainText to show info, rather use the second line

[Matej Mrenica]

mthw added a comment.


  Is everything fine now?

REPOSITORY
  R120 Plasma Workspace

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

To: mthw, ngraham, #vdg, #plasma, broulik, ndavis
Cc: ndavis, 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

D25792: Add notifiers to workspace options kcfg

[David Edmundson]

This revision was automatically updated to reflect the committed changes.
Closed by commit R119:8b00bcb9bc09: Add notifiers to workspace options kcfg 
(authored by davidedmundson).

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D25792?vs=71030=71155

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

AFFECTED FILES
  kcms/workspaceoptions/workspaceoptions_kdeglobalssettings.kcfgc

To: davidedmundson, ervin
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