D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Rajeesh K Nambiar
knambiar added a comment.


  +1.
  I've also spent some time debugging this issue, and was perplexed why the 
property change event doesn't propagate. This fix should be backported if 
possible.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  arcpatch-D10498

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

To: aacid, davidedmundson, xuetianweng
Cc: knambiar, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D3829: [touchpad] Use a separate X11 Display to monitor the XInput event.

2018-02-13 Thread Martin Flöser
graesslin added a comment.


  In D3829#158947 , @ngraham wrote:
  
  > @graesslin, does this still need changes to be merge-worthy?
  
  
  Yes, we did not migrate back to xlib, so the port to xcb is still required.

REPOSITORY
  R119 Plasma Desktop

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

To: xuetianweng, davidedmundson
Cc: aacid, z3ntu, ngraham, knambiar, graesslin, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10490: Add a method to dbus interface to query information about a window

2018-02-13 Thread Martin Flöser
graesslin added inline comments.
Restricted Application edited projects, added KWin; removed Plasma.

INLINE COMMENTS

> broulik wrote in dbusinterface.cpp:199
> Can you use introspection to fill this `QVariantMap` rather than manually 
> doing this?

We don't have properties for all of them and it's currently hand picked for 
what the kcm needs.

REPOSITORY
  R108 KWin

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

To: graesslin, #kwin, #plasma
Cc: davidedmundson, broulik, plasma-devel, kwin, iodelay, bwowk, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, 
apol, mart


D10495: Workaround to restore KF5 programs from system tray

2018-02-13 Thread Martin Flöser
graesslin added a comment.


  In D10495#205811 , @stikonas wrote:
  
  > This is more to start some discussion on system tray under Wayland. I'm not 
sure myself if this should be committed. I just use this workaround locally 
until winId() works on Wayland.
  
  
  winId will never work. I cannot say much on the patch as the context is 
missing.

REPOSITORY
  R289 KNotifications

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

To: stikonas, wbauer, #plasma, davidedmundson, volkov
Cc: graesslin, #kwin, plasma-devel, kde-frameworks-devel, #frameworks, 
michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, 
sebas, apol, mart


D10502: Eliminate unnecessary bottom pading on OverlaySheets

2018-02-13 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R169 Kirigami

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

To: ngraham, #kirigami, apol
Cc: plasma-devel, apol, davidedmundson, mart, hein


D10502: Eliminate unnecessary bottom pading on OverlaySheets

2018-02-13 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: Kirigami, apol.
Restricted Application added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  The OverlaySheet already defines a bottomMargin, so we don't need to add so 
much extra space.
  
  BUG: 390032

TEST PLAN
  Tested with Discover's review input sheet.
  
  Before:
  
  After:
  
  Also tested in Kirigami Gallery; all pop-ups I could find still looked good.

REPOSITORY
  R169 Kirigami

BRANCH
  less-popup-padding

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

AFFECTED FILES
  src/controls/templates/OverlaySheet.qml

To: ngraham, #kirigami, apol
Cc: plasma-devel, apol, davidedmundson, mart, hein


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham added a reviewer: abetts.
ngraham added a comment.


  Great, now we have the titles on the left, like @mart  wants. We're getting 
there! There are a couple remaining issues:
  
  - Needs a re-base as it doesn't apply cleanly onto current master
  - Now everything in the header is left-aligned. It looks okay in widescreen 
mode, but just doesn't work outside of widescreen mode: F5710867: Too much on 
the left 2.png  F5710868: Too much on the 
left 1.png  It looks so unbalanced. I 
really think we need to move the context actions over to the right. The left is 
overloaded with stuff.
  - When I merge master and resolve the merge conflicts, the context actions 
overlap the navigation buttons: F5710865: overlapping.png 


REPOSITORY
  R169 Kirigami

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

To: apol, #kirigami, mart, ngraham, abetts
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10461: GMenu-DBusMenu-Proxy

2018-02-13 Thread Konstantin
rilian added a comment.


  Hello, I am Konstantin, developer of vala-panel-appmenu. I have some comments 
about your application.
  
  MenuModel protocol consitsts for 5 items:
  
AppMenu - with property _GTK_APPMENU_OBJECT_PATH
MenuBar - with property _GTK_MENUBAR_OBJECT_PATH
It is a menu models, it is how menu should drawn on screen.
One can be missing, and then incomplete menu should render:
a) If AppMenu is missing, you will miss menu entry with application name 
(vala-panel-appmenu renders stub in place of missing AppMenu)
b) If MenuBar is missing, you will miss all menu entries except entry with 
application name.
c) If both are missing, you will not see a menu (Protocol is incorrect)
  
  And 3 providers of actions. Actions is required to get menu react on user 
changes.Providers:
  
Application (_GTK_APPLICATION_OBJECT_PATH, prefix app) - it is actions from 
all application (not bound to a particular window)
Window (_GTK_WINDOW_OBJECT_PATH, prefix win) - it is actions from current 
window, as it set by a developer of application
Unity (_UNITY_OBJECT_PATH, prefix unity) - it is non-standard, but widely 
used action path for set a Unity actions (when window actions is not supported 
by app developer). It is object path supported by unity-gtk-module and 
appmenu-gtk-module.
If you implement this, you will get GTK2 menu for free.
If any of this are missing, this menu items should be rendered as disabled. 
But if menu using actions only from one category - it can be used as a normal 
menu. Setting this all is not required for functional menu. One will be enough, 
if menu is using actions only from one group.

REPOSITORY
  R120 Plasma Workspace

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

To: broulik, #plasma
Cc: rilian, mtallur, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10500: Make the review pop-up a bit more user-friendly

2018-02-13 Thread Nathaniel Graham
ngraham edited the summary of this revision.
ngraham edited the test plan for this revision.

REPOSITORY
  R134 Discover Software Store

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

To: ngraham, #discover_software_store
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10500: Make the review pop-up a bit more user-friendly

2018-02-13 Thread Nathaniel Graham
ngraham created this revision.
ngraham added a reviewer: Discover Software Store.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  Polish the design of the review input pop-up, and make the button tell you 
what you need to do

TEST PLAN
  [images go here]

REPOSITORY
  R134 Discover Software Store

BRANCH
  review-popup-button-text

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

AFFECTED FILES
  discover/qml/ReviewDialog.qml

To: ngraham, #discover_software_store
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27129.
apol added a comment.


  Make the title actually stay to the left

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27083=27129

BRANCH
  master

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

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10499: [spellcheck runner] Make each suggestion copyable as separate item

2018-02-13 Thread Friedrich W . H . Kossebau
kossebau added inline comments.

INLINE COMMENTS

> spellcheck.cpp:222
> +Plasma::QueryMatch match(this);
> +match.setType(Plasma::QueryMatch::HelperMatch);
>  match.setIconName(QStringLiteral("checkbox"));

Not sure if HelperMatch is the correct thing here. Similar runners use 
InformationalMatch here, so on selection the term is copied into the runner.

What is the modern krunner way to support the use-case of someone wanting to 
know the spelling of some word, and then copy the correct or suggested into the 
clipboard for further usage?

REPOSITORY
  R114 Plasma Addons

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

To: kossebau, broulik
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10499: [spellcheck runner] Make each suggestion copyable as separate item

2018-02-13 Thread Friedrich W . H . Kossebau
kossebau created this revision.
kossebau added a reviewer: broulik.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Before all suggestions are only shown as single item with a text of
  comma-separated terms. So one cannot select a single suggested term and e.g.
  copy it to the clipboard
  
  And if correct term, the term is not copyable directly as well.

TEST PLAN
  Activate spellcheck runner (disabled by default), enter
  "spell Compilcated" or similar
  and select spellchecker menu items.

REPOSITORY
  R114 Plasma Addons

BRANCH
  spellrunnerMakeTermsCopyable

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

AFFECTED FILES
  runners/spellchecker/spellcheck.cpp

To: kossebau, broulik
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Albert Astals Cid
aacid added a comment.


  I updated the comment, can you have a look and approve again?

REPOSITORY
  R119 Plasma Desktop

BRANCH
  arcpatch-D10498

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

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Xuetian Weng
xuetianweng accepted this revision.
xuetianweng added a comment.


  I sent the comment before you corrected the diff branch. LGTM.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  arcpatch-D10498

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

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Albert Astals Cid
aacid updated this revision to Diff 27126.
aacid added a comment.


  better? explanation of why the workaround works

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10498?vs=27125=27126

BRANCH
  arcpatch-D10498

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

AFFECTED FILES
  kcms/touchpad/src/backends/x11/xlibbackend.cpp

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Xuetian Weng
xuetianweng accepted this revision.
xuetianweng added a comment.
This revision is now accepted and ready to land.


  Oh, ok.. now the diff looks normal.

REPOSITORY
  R119 Plasma Desktop

BRANCH
  arcpatch-D10498

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

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Xuetian Weng
xuetianweng added a comment.


  Emm, mind to get rid of the .desktop part for this review?
  
  Also I think you need to give some explanation about why this works.

REPOSITORY
  R119 Plasma Desktop

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

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Albert Astals Cid
aacid updated this revision to Diff 27125.
aacid added a comment.


  correct diff against Plasma/5.12

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10498?vs=27123=27125

BRANCH
  arcpatch-D10498

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

AFFECTED FILES
  kcms/touchpad/src/backends/x11/xlibbackend.cpp

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Albert Astals Cid
aacid updated this revision to Diff 27123.
aacid edited the summary of this revision.
aacid added a comment.


  [try to] add BUGS:

REPOSITORY
  R119 Plasma Desktop

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10498?vs=27122=27123

BRANCH
  arcpatch-D10498

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

AFFECTED FILES
  applets/icontasks/metadata.desktop
  applets/kicker/package/contents/ui/DashboardRepresentation.qml
  applets/kicker/package/contents/ui/DashboardTabBar.qml
  applets/kicker/plugin/dashboardwindow.cpp
  applets/taskmanager/package/metadata.desktop
  kcms/access/kcmaccess.desktop
  kcms/autostart/autostart.desktop
  kcms/componentchooser/componentservices/kcm_filemanager.desktop
  kcms/desktoppaths/desktoppath.desktop
  kcms/fonts/fonts.desktop
  kcms/input/mouse.desktop
  kcms/launch/kcmlaunch.desktop
  kcms/nightcolor/kcm_nightcolor.desktop
  kcms/nightcolor/package/metadata.desktop
  kcms/touchpad/src/backends/x11/xlibbackend.cpp
  layout-templates/org.kde.plasma.desktop.appmenubar/metadata.desktop
  layout-templates/org.kde.plasma.desktop.defaultPanel/metadata.desktop
  layout-templates/org.kde.plasma.desktop.emptyPanel/metadata.desktop
  org.kde.plasmashell.metainfo.xml

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Albert Astals Cid
aacid added reviewers: davidedmundson, xuetianweng.

REPOSITORY
  R119 Plasma Desktop

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

To: aacid, davidedmundson, xuetianweng
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D3829: [touchpad] Use a separate X11 Display to monitor the XInput event.

2018-02-13 Thread Albert Astals Cid
aacid added a comment.


  My proposal to fix the bug at https://phabricator.kde.org/D10498

REPOSITORY
  R119 Plasma Desktop

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

To: xuetianweng, davidedmundson
Cc: aacid, z3ntu, ngraham, knambiar, graesslin, plasma-devel, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10498: Workaround the touchpad toggle button not working

2018-02-13 Thread Albert Astals Cid
aacid created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
aacid requested review of this revision.

TEST PLAN
  Without it the touchpad toggle at my laptop just toggles once
  now it works fine all the time

REPOSITORY
  R119 Plasma Desktop

BRANCH
  Plasma/5.12

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

AFFECTED FILES
  kcms/touchpad/src/backends/x11/xlibbackend.cpp

To: aacid
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


Re: D10489: Fix the tray icon scaling

2018-02-13 Thread Aleix Pol
For some reason this request is hidden?

On Tue, Feb 13, 2018 at 8:05 PM, Piotr Kosinski  wrote:

> pgkos created this revision.
> Restricted Application added a project: Plasma.
> Restricted Application added a subscriber: plasma-devel.
> pgkos requested review of this revision. View Revision
> 
> *REPOSITORY*
> R120 Plasma Workspace
>
> *BRANCH*
> tray-icon-size-fix
>
> *REVISION DETAIL*
> https://phabricator.kde.org/D10489
>
> *AFFECTED FILES*
> applets/systemtray/package/contents/ui/main.qml
>
> *To: *pgkos
> *Cc: *plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed,
> jensreuterberg, abetts, sebas, apol, mart
>


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag added a comment.


  F5710626: with-check.png 
  
  F5710628: no-check.png 
  
  F5710630: no-icons.png 

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag updated this revision to Diff 27116.
zzag added a comment.


  - try to align check boxes by increasing MenuItem_MarginWidth
  - fix another double space problem

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10480?vs=27072=27116

BRANCH
  center-checkbox

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

AFFECTED FILES
  kstyle/breeze.h
  kstyle/breezestyle.cpp

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10495: Workaround to restore KF5 programs from system tray

2018-02-13 Thread Wolfgang Bauer
wbauer added a subscriber: KWin.
wbauer added a comment.


  Indeed.
  One thing that started this is https://bugs.kde.org/show_bug.cgi?id=389663
  
  I do think this should be fixed on the lower level if possible.
  
  Adding KWin as subscriber as well to hopefully get most people that have more 
insight.

REPOSITORY
  R289 KNotifications

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

To: stikonas, wbauer, #plasma, davidedmundson, volkov
Cc: #kwin, plasma-devel, kde-frameworks-devel, #frameworks, michaelh, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag added a comment.


  F5710509: a.png 
  
  //Side note: This patch tries to center every checkbox between the left 
border and blue rect(and it also ignores margins.)//
  
  F5710580: Screenshot_20180214_002713.png 

  
  Why are there only 2 pixels?
  
  After playing a bit with margin values I've found 5 to be reasonable.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos updated this revision to Diff 27114.
pgkos added a comment.


  Use units.smallSpacing for padding instead of magic numbers.

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10487?vs=27105=27114

BRANCH
  tray-icon-size-fix

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

AFFECTED FILES
  applets/icon/package/contents/ui/main.qml
  applets/systemtray/package/contents/ui/main.qml

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  Padding is fine. We're just saying that:
  
  - You need to retain the default appearance for a default sized panel
  - You need to use programmatic padding values rather than magic numbers

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos added a comment.


  I have tried and tested this diff without any padding, and trust me, it looks 
very bad - **I would rather have this patch not accepted** - than not to add 
any padding.
  
  Particularly, the tray icons look extremely bad when there is no padding - I 
suppose this is because the SVG icons are sized a little differently than 
pixmap icons.
  
  For example, the KDE Connect icon almost doesn't fit in the panel when it has 
no padding.
  
  I will try with units.smallSpacing.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread David Edmundson
davidedmundson added a comment.


  Yeah, the clock code isn't something I'd accept either.
  
  > But with your attitude it will be difficult to justify anything.
  
  Plasma has a predefined margin size smallSpacing. If you're using something 
else, it probably isnt' going to end up consistent.
  
  You'll also find iconItem which the Icon plasmoid uses visually will pad 
icons to the nearest icon size internally to the QQuickItem.
  Some Plasmoid's compact representation is a striaght IconItem, some aren't, 
which is why systray forces this to make them more uniform.
  
  When testing make sure you have the notification applet visible, and some 
SNIs.
  
  -
  
  All I'm asking for is, if the intention is to make them the same as something 
else, we need to make them actually programatically match in a way that works 
across any settings any user might have.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  In D10487#205827 , @pgkos wrote:
  
  > I could argue that the numbers come from Material Design guidelines 
 - it 
recommends between 10% to 20% of padding around icons.
  >
  > But with your attitude it will be difficult to justify anything.
  
  
  And that //that// attitude, you won't get a lot of patches accepted. :) 
Material Design is for Microsoft's platform, not ours. Let's not try to change 
multiple things in one patch. You may be able to get support for making the 
icons scale as the Panel is increased in height, but if you want to change the 
default padding, do that in another patch.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos added a comment.


  I could argue that the numbers come from Material Design guidelines 
 - it 
recommends between 10% to 20% of padding around icons.
  
  But with your attitude it will be difficult to justify anything.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos added a comment.


  So maybe you tell me where this comes from? 
https://cgit.kde.org/plasma-workspace.git/commit/applets/digital-clock/package/contents/ui/DigitalClock.qml?id=90ea27f49c84f0dffbbf79b29eaa14c57f31a24d

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10495: Workaround to restore KF5 programs from system tray

2018-02-13 Thread Andrius Štikonas
stikonas added a comment.


  This is more to start some discussion on system tray under Wayland. I'm not 
sure myself if this should be committed. I just use this workaround locally 
until winId() works on Wayland.

REPOSITORY
  R289 KNotifications

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

To: stikonas, wbauer, #plasma, davidedmundson, volkov
Cc: plasma-devel, kde-frameworks-devel, #frameworks, michaelh, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10495: Workaround to restore KF5 programs from system tray

2018-02-13 Thread Andrius Štikonas
stikonas created this revision.
stikonas added reviewers: wbauer, Plasma, davidedmundson, volkov.
Restricted Application added projects: Plasma, Frameworks.
Restricted Application added subscribers: Frameworks, plasma-devel.
stikonas requested review of this revision.

REVISION SUMMARY
  Currently restoring system tray does not work on Wayland.
  
  Even with this commit, the functionality is still worse than on X11 because 
clicking on system tray does not hide application window.

REPOSITORY
  R289 KNotifications

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

AFFECTED FILES
  src/kstatusnotifieritem.cpp

To: stikonas, wbauer, #plasma, davidedmundson, volkov
Cc: plasma-devel, kde-frameworks-devel, #frameworks, michaelh, ZrenBot, 
progwolff, lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread David Edmundson
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  I'm not merging a patch that just adds random numbers that you happen to 
think look good.
  
  It's not unification if you can't justify where the numbers come from in 
relation to other bits of code.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos added a comment.


  I have tested it with the panel in both horizontal and vertical orientations, 
and 0.85 looks optimal for me.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos updated this revision to Diff 27105.
pgkos added a comment.


  Squashed the commits

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10487?vs=27103=27105

BRANCH
  tray-icon-size-fix

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

AFFECTED FILES
  applets/icon/package/contents/ui/main.qml
  applets/systemtray/package/contents/ui/main.qml

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10480: align checkable widgets in menu items

2018-02-13 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment.


  In D10480#205772 , @zzag wrote:
  
  > > In principle it should not be needed. Right ?
  >
  > Yes.
  >
  > > Ifcheckboxes have a fixed size, and if margins would be equal to spacing, 
and if the math are correct, having something laied out as: margin + checkbox + 
spacing + icon + spacing + text would be automatically centered, no ?
  >
  > Well, in theory, yes, it would be centered. But, humans often perceive 
something to be centered when it's not actually centered, for example, in 
typography, letters like "O" or "S" are often put below a baseline. So, maybe, 
checkboxes should be even a little bit closer to icons/text.
  >
  > //Edit
  >  To make things clear: I'm not telling "checkboxes should be closer to 
icons.//
  >
  > At a given moment, here's how things laid out:
  >
  >   margin + checkbox_size + spacing + 1 + [2 + PM_SmallIconSize + 2] + 
spacing + 1 + text
  >
  >
  > Also, why is there the number 1?
  
  
  Yes. That's what needs fixing. I know there are subtleties with rect.left + 
rect.width() vs rect.right(). You can easily loose or gain a +1 there. 
  Also, fonts might not start exactly at the rect depending on hinting, which 
letter is drawn etc. 
  But anyway. This can be investigated, if only by actually drawing the 
relevant rect and see where space is lost, misaligned, etc.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos updated this revision to Diff 27103.
pgkos added a comment.


  Make the padding of all the panel icons the same

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10487?vs=27095=27103

BRANCH
  tray-icon-size-fix

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

AFFECTED FILES
  applets/icon/package/contents/ui/main.qml
  applets/systemtray/package/contents/ui/main.qml

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag added a comment.


  > In principle it should not be needed. Right ?
  
  Yes.
  
  > Ifcheckboxes have a fixed size, and if margins would be equal to spacing, 
and if the math are correct, having something laied out as: margin + checkbox + 
spacing + icon + spacing + text would be automatically centered, no ?
  
  Well, in theory, yes, it would be centered. But, humans often perceive 
something to be centered when it's not actually centered, for example, in 
typography, letters like "O" or "S" are often put below a baseline. So, maybe, 
checkboxes should be even a little bit closer to icons/text.
  
  At a given moment, here's how things laid out:
  
margin + checkbox_size + spacing + 1 + [2 + PM_SmallIconSize + 2] + spacing 
+ 1 + text
  
  Also, why is there the number 1?
  
  > The reason why I would rather fix the current code than introducing some 
centering, is because centering would automatically hide deeper problems, and 
because it makes the metrics (margin, icon width, spacing), not reflect what 
the code do. Which then introduces confusion and make the code not maintainable.
  
  Agreed.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10490: Add a method to dbus interface to query information about a window

2018-02-13 Thread David Edmundson
davidedmundson added a comment.
Restricted Application edited projects, added Plasma; removed KWin.


  Cool approach. One minor thing that won't happen anyway.

INLINE COMMENTS

> dbusinterface.cpp:193
> +{
> +m_replyQueryWindowInfo = message();
> +setDelayedReply(true);

can you add:

if (m_replyQuery...) {sendError()}

otherwise if a user makes two calls without selecting a window inbetween the 
first call will just never get a response

> detectwidget.cpp:152
> +  
> QStringLiteral("queryWindowInfo"));
> +QDBusPendingReply async = 
> QDBusConnection::sessionBus().asyncCall(message);
> +

I would explicilty specify a massive timeout for anything which has user 
interaction the other side.

> ruleswidget.cpp:790
> +CHECKBOX_PREFILL(below, , info.value("keepBelow").toBool());
> +// noborder is only internal KWin information, so let's guess
> +CHECKBOX_PREFILL(noborder, , info.value("noBorder").toBool());

This isn't a guess anymore?

REPOSITORY
  R108 KWin

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

To: graesslin, #kwin, #plasma
Cc: davidedmundson, broulik, plasma-devel, kwin, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, jensreuterberg, abetts, sebas, apol, mart, iodelay, 
bwowk, hardening


D10490: Add a method to dbus interface to query information about a window

2018-02-13 Thread Kai Uwe Broulik
broulik added inline comments.
Restricted Application edited projects, added KWin; removed Plasma.

INLINE COMMENTS

> dbusinterface.cpp:199
> +const QVariantMap ret{
> +{QStringLiteral("resourceClass"), c->resourceClass()},
> +{QStringLiteral("resourceName"), c->resourceName()},

Can you use introspection to fill this `QVariantMap` rather than manually doing 
this?

REPOSITORY
  R108 KWin

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

To: graesslin, #kwin, #plasma
Cc: broulik, plasma-devel, kwin, iodelay, bwowk, ZrenBot, progwolff, 
lesliezhai, ali-mohamed, hardening, jensreuterberg, abetts, sebas, apol, mart


D10438: reserve space for checkable widgets in menu items

2018-02-13 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment.


  In D10438#205519 , @zzag wrote:
  
  > Maybe an option like "Reserve space for check boxes" should be added in 
fine tuning settings?
  
  
  Please no option. If we put an option to every feature we are not sure about, 
then we end up with unmaintainable and cluttered code, and only offer a 
cluttered style to the user.
  Either we like this feature and it stays, or we dont and it goes.
  
  ... another screenshot: 
  F5710404: Screenshot_20180213_205409.png 


REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, ngraham, hpereiradacosta
Cc: alake, colomar, januz, fabianr, mmustac, abetts, anemeth, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart


D10480: align checkable widgets in menu items

2018-02-13 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment.


  Ok. So, the patch works, but ... there is something fishy here. In principle 
it should not be needed. Right ? 
  Ifcheckboxes have a fixed size, and if margins would be equal to spacing, and 
if the math are correct, having something laied out as: margin + checkbox + 
spacing + icon + spacing + text would be automatically centered, no ? 
  Idem without icon. (and if there is double spacing, then it is a bug).
  At least this is how the code is supposed to work. If not, it should be 
fixed, rather than circomvoluting the issue with some centering. 
  (note that the padding on the icons is disconnected to this: not all icons 
have padding, especially if you change icon theme, so you cannot do anything 
but rely on the icon size.
  
  Bottom line, I would like to investigate the current code (which _should_ 
work), rather than accepting this patch. 
  Or do I miss something ?
  
  The reason why I would rather fix the current code than introducing some 
centering, is because centering would automatically hide deeper problems, and 
because it makes the metrics (margin, icon width, spacing), not reflect what 
the code do. Which then introduces confusion and make the code not maintainable.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Nathaniel Graham
This revision was automatically updated to reflect the committed changes.
Closed by commit R169:667cc4318a18: Polish ToolBarApplicationHeader appearance 
(authored by ngraham).

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10483?vs=27096=27097

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

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ForwardButton.qml

To: ngraham, #kirigami, mart
Cc: abetts, #discover_software_store, plasma-devel, apol, davidedmundson, mart, 
hein


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Marco Martin
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R169 Kirigami

BRANCH
  header-polish

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

To: ngraham, #kirigami, mart
Cc: abetts, #discover_software_store, plasma-devel, apol, davidedmundson, mart, 
hein


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread David Edmundson
davidedmundson requested changes to this revision.
davidedmundson added a comment.
This revision now requires changes to proceed.


  > The 0.8 is used to create a small padding, so the icons never touch the 
panel's borders.
  
  If the goal of the patch is to make the icon sizes the same as the icon in 
some other bit of the panel, then surely the size code needs to be identical to 
that other bit of code. I can't see the 0.8 anywhere else.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces, davidedmundson
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  I fear this will generate the exact same types of user complaints that the 
huge clock text did before we gave it more vertical padding: people will say 
that the icons are too tall and don't have enough padding. Let's try not 
changing the appearance and vertical padding for default-height panels and just 
focus on making sure that they scale well as the panel is increased in size.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos added a comment.


  This patch also enables the user to configure the tray icon's size limit 
using System Settings -> Icons -> Advanced -> Panel

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  I've reverted the title alignment changes for now. There are too many patches 
in flight to keep up! If this is good, let's land it (and also Aleix's patches 
once they're all good too) and then I'll iterate on the titles a bit more. Does 
that sound like a plan?

REPOSITORY
  R169 Kirigami

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

To: ngraham, #kirigami, mart
Cc: abetts, #discover_software_store, plasma-devel, apol, davidedmundson, mart, 
hein


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R169 Kirigami

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

To: ngraham, #kirigami, mart
Cc: abetts, #discover_software_store, plasma-devel, apol, davidedmundson, mart, 
hein


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos added a comment.


  The 0.8 is used to create a small padding, so the icons never touch the 
panel's borders.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Nathaniel Graham
ngraham updated this revision to Diff 27096.
ngraham added a comment.


  Revert change to center title

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10483?vs=27071=27096

BRANCH
  header-polish

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

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ForwardButton.qml

To: ngraham, #kirigami, mart
Cc: abetts, #discover_software_store, plasma-devel, apol, davidedmundson, mart, 
hein


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos edited the summary of this revision.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10485: [lookandfeel kcm] Do not declare plugin in lookandfeeltool code version

2018-02-13 Thread David Edmundson
davidedmundson accepted this revision.
davidedmundson added a comment.
This revision is now accepted and ready to land.


  Thanks!
  
  please close 389982

REPOSITORY
  R119 Plasma Desktop

BRANCH
  definenopluginwithlookandfeeltool

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

To: kossebau, #freebsd, tcberner, bshah, mart, davidedmundson
Cc: davidedmundson, rikmills, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos updated this revision to Diff 27095.
pgkos added a comment.


  Fix tray icon scaling

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10487?vs=27089=27095

BRANCH
  tray-icon-size-fix

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

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

To: pgkos, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  For me this patch still has the titles aligned to the right, even after your 
change:
  
  F5710374: right.png 
  
  But as currently implemented, if we move the title over to the left as I'm 
asking for, then the above image would change too having **three** elements on 
the left, and **none** on the right. It would look visually unbalanced.
  
  I think the context items need to be moved over to the right side of the 
header to visually balance the navigation buttons and title that will go on the 
left.

REPOSITORY
  R169 Kirigami

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

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


[Breeze] [Bug 364849] Tooltip Text in several qt apps such as Amarok and Krusader is unreadable (white on white) with the Breeze/Breeze Dark color schemes

2018-02-13 Thread Myriam Schweingruber
https://bugs.kde.org/show_bug.cgi?id=364849

Myriam Schweingruber  changed:

   What|Removed |Added

 Status|UNCONFIRMED |RESOLVED
 Resolution|--- |FIXED

--- Comment #6 from Myriam Schweingruber  ---
Closing as solved in Plasma 5.12.0, see comment #5

-- 
You are receiving this mail because:
You are the assignee for the bug.

D10490: Add a method to dbus interface to query information about a window

2018-02-13 Thread Martin Flöser
graesslin created this revision.
graesslin added reviewers: KWin, Plasma.
Restricted Application added a project: KWin.
Restricted Application added subscribers: kwin, plasma-devel.
graesslin requested review of this revision.
Restricted Application edited projects, added Plasma; removed KWin.

REVISION SUMMARY
  This call is added for the window rules kcm which has a detect
  functionality. As that detect functionality cannot query any Wayland
  windows we need to have some functionality in KWin core. Furthermore
  this allows to simplify the code in the kcm as all the custom X11
  interaction can be removed. KWin internally has the functionality to
  find a window at a given position.
  
  From a security perspective adding this dbus method is fine as the user
  stays in control of the functionality. It requires active click to
  select a window.
  
  The new dbus call is already used in the rules kcm replacing the
  X11 based detect functionality. That a detect is now able to get
  information for both X11 and Wayland windows. So far only X11 windows
  on X11 were supported. So this fills an important gap in the Wayland
  offerings. It should now be possible to create rules for Wayland
  windows (though may not be fully functional).

TEST PLAN
  Run the kwin_rules_dialog and it detected the window correctly

REPOSITORY
  R108 KWin

BRANCH
  rules-detect-dbus

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

AFFECTED FILES
  dbusinterface.cpp
  dbusinterface.h
  kcmkwin/kwinrules/CMakeLists.txt
  kcmkwin/kwinrules/detectwidget.cpp
  kcmkwin/kwinrules/detectwidget.h
  kcmkwin/kwinrules/ruleswidget.cpp
  kcmkwin/kwinrules/ruleswidget.h
  org.kde.KWin.xml

To: graesslin, #kwin, #plasma
Cc: plasma-devel, kwin, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos edited the test plan for this revision.

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10489: Fix the tray icon scaling

2018-02-13 Thread Piotr Kosinski
pgkos created this revision.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
pgkos requested review of this revision.

REPOSITORY
  R120 Plasma Workspace

BRANCH
  tray-icon-size-fix

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

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

To: pgkos
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10471: Use Kirgami.ToolBarApplicationHeader instead of the view headers

2018-02-13 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R134 Discover Software Store

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

To: apol, ngraham
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  Thanks for the patch!
  
  1. Please follow the formatting guidelines in 
https://community.kde.org/Infrastructure/Phabricator#Formatting_your_patch
  2. Please update the Test Plan section with evidence of testing. A pair of 
before-and-after screenshots is always very nice (and may get you featured in 
next weekend's Usability & Productivity blog post 
)

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces
Cc: ngraham, davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread David Edmundson
davidedmundson added a comment.


  Which application icons, and where are they set to 0.8?

REPOSITORY
  R120 Plasma Workspace

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

To: pgkos, #plasma_workspaces
Cc: davidedmundson, plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D10487: Fix tray icon size scaling when changing the panel size (fix bug 360333)

2018-02-13 Thread Piotr Kosinski
pgkos created this revision.
pgkos added a reviewer: Plasma: Workspaces.
pgkos added a project: Plasma: Workspaces.
Restricted Application edited projects, added Plasma; removed Plasma: 
Workspaces.
Restricted Application added a subscriber: plasma-devel.
pgkos requested review of this revision.

REVISION SUMMARY
  This fixes the bug 360333  by 
making the tray icons scale exactly like the application icons on the left.

REPOSITORY
  R120 Plasma Workspace

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

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

To: pgkos, #plasma_workspaces
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  No, but in Desktop mode, if the window is not widescreen, then the left side 
of the header currently gets navigation buttons: Back when you can go back, and 
after the first time you go back, Forward is shown, too.

REPOSITORY
  R169 Kirigami

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

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol added a comment.


  In D10475#205609 , @ngraham wrote:
  
  > Also, what happens to the navigation buttons if you're not in widescreen 
view? You get navigation buttons, and then context actions, and then the title? 
That'll be an awful lot of content to have on the left side.
  
  
  When on a mobile device, we should not use this toolbar.

REPOSITORY
  R169 Kirigami

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

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27083.
apol added a comment.


  Keep the title aligned to the left

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27074=27083

BRANCH
  master

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

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham requested changes to this revision.
ngraham added a comment.
This revision now requires changes to proceed.


  This patch makes the title right-aligned, which is non-standard with 
anything. Let's keep it left-aligned for now. Also, what happens to the 
navigation buttons if you're not in widescreen view? You get navigation 
buttons, and then context actions, and then the title? That'll be an awful lot 
of content to have on the left side.

REPOSITORY
  R169 Kirigami

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

To: apol, #kirigami, mart, ngraham
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10471: Use Kirgami.ToolBarApplicationHeader instead of the view headers

2018-02-13 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27080.
apol added a comment.


  Fix the search page

REPOSITORY
  R134 Discover Software Store

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10471?vs=27034=27080

BRANCH
  master

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

AFFECTED FILES
  discover/qml/ApplicationPage.qml
  discover/qml/ApplicationsListPage.qml
  discover/qml/BrowsingPage.qml
  discover/qml/DiscoverWindow.qml
  discover/qml/InstallApplicationButton.qml
  discover/qml/InstalledPage.qml
  discover/qml/KirigamiActionBridge.qml
  discover/qml/SearchPage.qml
  discover/qml/SourcesPage.qml
  discover/resources.qrc

To: apol, ngraham
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10485: [lookandfeel kcm] Do not declare plugin in lookandfeeltool code version

2018-02-13 Thread Friedrich W . H . Kossebau
kossebau added a reviewer: mart.

REPOSITORY
  R119 Plasma Desktop

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

To: kossebau, #freebsd, tcberner, bshah, mart
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Marco Martin
mart added a comment.


  In D10483#205563 , @abetts wrote:
  
  > Sorry guys, this proposed left alignment just won't work IMHO. The reason 
is that we are turning the bar into a CSD. Unlike a desktop application, we 
don't have the ability to shrink the application in mobile. What this amounts 
to is that people will want to add more controls to the top bar, this will cram 
the interface and the limited space will run out. Pushing the title to the left 
doesn't work either, it will bump into the back button. I feel it is best that 
we drop this conversation for now and focus on the HIG first where we can 
address these ideas first and then try to adjust applications to it.
  
  
  this look is strictly for desktop:
  on mobile, the top bar contents are completely different, is a breadcrumb
  and the actions are floating at bottom
  
  F5710089: Spectacle.S31721.png 

REPOSITORY
  R169 Kirigami

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

To: ngraham, #kirigami, mart
Cc: abetts, #discover_software_store, plasma-devel, apol, davidedmundson, mart, 
hein


D10485: [lookandfeel kcm] Do not declare plugin in lookandfeeltool code version

2018-02-13 Thread Friedrich W . H . Kossebau
kossebau created this revision.
kossebau added reviewers: FreeBSD, tcberner, bshah.
Restricted Application added a project: Plasma.
Restricted Application added a subscriber: plasma-devel.
kossebau requested review of this revision.

REVISION SUMMARY
  Also improves race condition on creating the JSON file and running
  automoc over the cpp file which refers to it, given there is no
  dependency chain defined at all for the lookandfeeltool target and
  that kcm_lookandfeel.json, which was prone to make highly parallel
  builds fail

REPOSITORY
  R119 Plasma Desktop

BRANCH
  definenopluginwithlookandfeeltool

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

AFFECTED FILES
  kcms/lookandfeel/CMakeLists.txt
  kcms/lookandfeel/kcm.cpp

To: kossebau, #freebsd, tcberner, bshah
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D7849: Fix the tray icon scaling on HiDPI screens

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  Does this actually fully fix https://bugs.kde.org/show_bug.cgi?id=360333?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: pgkos, #plasma
Cc: aspotashev, ngraham, anthonyfieroni, broulik, #frameworks, davidedmundson, 
plasma-devel, michaelh, ZrenBot, progwolff, lesliezhai, ali-mohamed, 
jensreuterberg, abetts, sebas, apol, mart


D10461: GMenu-DBusMenu-Proxy

2018-02-13 Thread Kai Uwe Broulik
broulik updated this revision to Diff 27077.
broulik retitled this revision from "WIP: GMenu-DBusMenu-Proxy" to 
"GMenu-DBusMenu-Proxy".
broulik edited the summary of this revision.
broulik added a comment.


  - Cleanup code and use categorized logging
  - Monitor action changes (e.g. toggled state of checkbox)
  - Monitor menu changes (e.g. menu label changed)
  - Handle when app has no menu on startup but gets one later
  - Disable GTK menu bar when we're running

REPOSITORY
  R120 Plasma Workspace

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10461?vs=26999=27077

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

AFFECTED FILES
  CMakeLists.txt
  gmenu-dbusmenu-proxy/CMakeLists.txt
  gmenu-dbusmenu-proxy/gdbusmenutypes_p.cpp
  gmenu-dbusmenu-proxy/gdbusmenutypes_p.h
  gmenu-dbusmenu-proxy/gmenudbusmenuproxy.desktop
  gmenu-dbusmenu-proxy/main.cpp
  gmenu-dbusmenu-proxy/menu.cpp
  gmenu-dbusmenu-proxy/menu.h
  gmenu-dbusmenu-proxy/menuproxy.cpp
  gmenu-dbusmenu-proxy/menuproxy.h

To: broulik, #plasma
Cc: mtallur, ngraham, plasma-devel, ZrenBot, progwolff, lesliezhai, 
ali-mohamed, jensreuterberg, abetts, sebas, apol, mart


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Marco Martin
mart accepted this revision.
This revision is now accepted and ready to land.

REPOSITORY
  R169 Kirigami

BRANCH
  master

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

To: apol, #kirigami, mart
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Aleix Pol Gonzalez
apol updated this revision to Diff 27074.
apol added a comment.


  Have the 3-dots button rightmost

REPOSITORY
  R169 Kirigami

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10475?vs=27033=27074

BRANCH
  master

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

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml

To: apol, #kirigami
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Andres Betts
abetts added a comment.


  Sorry guys, this proposed left alignment just won't work IMHO. The reason is 
that we are turning the bar into a CSD. Unlike a desktop application, we don't 
have the ability to shrink the application in mobile. What this amounts to is 
that people will want to add more controls to the top bar, this will cram the 
interface and the limited space will run out. Pushing the title to the left 
doesn't work either, it will bump into the back button. I feel it is best that 
we drop this conversation for now and focus on the HIG first where we can 
address these ideas first and then try to adjust applications to it.

REPOSITORY
  R169 Kirigami

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

To: ngraham, #kirigami, mart
Cc: abetts, #discover_software_store, plasma-devel, apol, davidedmundson, mart, 
hein


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R169 Kirigami

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

To: apol, #kirigami
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10471: Use Kirgami.ToolBarApplicationHeader instead of the view headers

2018-02-13 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R134 Discover Software Store

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

To: apol, ngraham
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Nathaniel Graham
ngraham edited the summary of this revision.

REPOSITORY
  R169 Kirigami

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

To: ngraham, #kirigami, mart
Cc: #discover_software_store, plasma-devel, apol, davidedmundson, mart, hein


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag updated this revision to Diff 27072.
zzag added a comment.


  fix checkbox aligning when there is no any icon

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10480?vs=27062=27072

BRANCH
  center-checkbox

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


Plasma 5.12.1

2018-02-13 Thread Jonathan Riddell
Plasma 5.12.1 is now out for your compilers

https://www.kde.org/announcements/plasma-5.12.1.php


D10438: reserve space for checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag added a comment.


  Alright here's a patch which would make possible to turn off reserving space. 
Yet, a checkbox should be added in "Fine tuning" tab.
  
From 45be224b2fb31966a3117aa1f226f8ea711f8109 Mon Sep 17 00:00:00 2001
From: Vlad Zagorodniy 
Date: Tue, 13 Feb 2018 17:44:43 +0200
Subject: [PATCH] add reserve space from checkboxes option

---
 kstyle/breezestyle.cpp | 20 +---
 kstyle/breezestyle.h   |  3 +++
 2 files changed, 20 insertions(+), 3 deletions(-)

diff --git a/kstyle/breezestyle.cpp b/kstyle/breezestyle.cpp
index 07f0be7d..02189575 100644
--- a/kstyle/breezestyle.cpp
+++ b/kstyle/breezestyle.cpp
@@ -2728,7 +2728,11 @@ namespace Breeze
 leftColumnWidth += Metrics::MenuItem_ItemSpacing;
 
 // add checkbox indicator width
-leftColumnWidth += Metrics::CheckBox_Size + 
Metrics::MenuItem_ItemSpacing;
+if( menuItemOption->menuHasCheckableItems || 
reserveSpaceForCheckboxesInMenus() )
+{
+leftColumnWidth += Metrics::CheckBox_Size
++  Metrics::MenuItem_ItemSpacing;
+}
 
 // add spacing for accelerator
 /*
@@ -4697,8 +4701,11 @@ namespace Breeze
 // define relevant rectangles
 // checkbox
 QRect checkBoxRect;
-checkBoxRect = QRect( contentsRect.left(), contentsRect.top() + 
(contentsRect.height()-Metrics::CheckBox_Size)/2, Metrics::CheckBox_Size, 
Metrics::CheckBox_Size );
-contentsRect.setLeft( checkBoxRect.right() + 
Metrics::MenuItem_ItemSpacing + 1 );
+if( menuItemOption->menuHasCheckableItems || 
reserveSpaceForCheckboxesInMenus() )
+{
+checkBoxRect = QRect( contentsRect.left(), contentsRect.top() 
+ (contentsRect.height()-Metrics::CheckBox_Size)/2, Metrics::CheckBox_Size, 
Metrics::CheckBox_Size );
+contentsRect.setLeft( checkBoxRect.right() + 
Metrics::MenuItem_ItemSpacing + 1 );
+}
 
 // render checkbox indicator
 if( menuItemOption->checkType == 
QStyleOptionMenuItem::NonExclusive )
@@ -7074,6 +7081,13 @@ namespace Breeze
 return g.readEntry("ShowIconsInMenuItems", true);
 }
 
+//
+bool Style::reserveSpaceForCheckboxesInMenus() const
+{
+const KConfigGroup g(KSharedConfig::openConfig(), "KDE");
+return g.readEntry("ReserveSpaceForCheckboxesInMenus", true);
+}
+
 //
 bool Style::showIconsOnPushButtons() const
 {
diff --git a/kstyle/breezestyle.h b/kstyle/breezestyle.h
index 12548c92..031a1225 100644
--- a/kstyle/breezestyle.h
+++ b/kstyle/breezestyle.h
@@ -468,6 +468,9 @@ namespace Breeze
 //* return true if icons should be shown in menus
 bool showIconsInMenuItems() const;
 
+//* return true if space is reserved for checkboxes in menus
+bool reserveSpaceForCheckboxesInMenus() const;
+
 //* return true if icons should be shown on buttons
 bool showIconsOnPushButtons() const;
 
-- 
2.16.1
  
  But, I would like to have D10480  in 
master so I could complete the patch above.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, ngraham, hpereiradacosta
Cc: alake, colomar, januz, fabianr, mmustac, abetts, anemeth, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Nathaniel Graham
ngraham edited the test plan for this revision.

REPOSITORY
  R169 Kirigami

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

To: ngraham, #kirigami, mart
Cc: #discover_software_store, plasma-devel, apol, davidedmundson, mart, hein


D10483: Polish ToolBarApplicationHeader appearance

2018-02-13 Thread Nathaniel Graham
ngraham created this revision.
ngraham added reviewers: Kirigami, mart.
Restricted Application added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.
ngraham requested review of this revision.

REVISION SUMMARY
  - Increase the toolbar height by 4 pixels, to make it look less cramped and 
give items inside it more vertical padding
  - Horizontally center the title
  - Give the navigation buttons some padding

TEST PLAN
  Tested with Discover.
  
  Before:
  
  After:

REPOSITORY
  R169 Kirigami

BRANCH
  header-polish

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

AFFECTED FILES
  src/controls/ToolBarApplicationHeader.qml
  src/controls/templates/private/BackButton.qml
  src/controls/templates/private/ForwardButton.qml

To: ngraham, #kirigami, mart
Cc: #discover_software_store, plasma-devel, apol, davidedmundson, mart, hein


D10438: reserve space for checkable widgets in menu items

2018-02-13 Thread Marijo Mustac
mmustac added a comment.


  I am a user :-D
  To be honest I liked Vlad`s proposal after my first comment a lot. { 
margin-left: 10px; }
  The current implementation is nothing I would really like to have and be 
definitly a reason to search for a solution or another theme (which would 
propably be the easier part for most of the poeple, specially new ones).

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, ngraham, hpereiradacosta
Cc: alake, colomar, januz, fabianr, mmustac, abetts, anemeth, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart


D10438: reserve space for checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag added a comment.


  Maybe an option like "Reserve space for check boxes" should be added in fine 
tuning settings?

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, ngraham, hpereiradacosta
Cc: alake, colomar, januz, fabianr, mmustac, abetts, anemeth, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart


D10438: reserve space for checkable widgets in menu items

2018-02-13 Thread Hugo Pereira Da Costa
hpereiradacosta added a comment.


  In D10438#205505 , @ngraham wrote:
  
  > FWIW I like it, but since this is mostly subjective, I wouldn't fight to 
the death to retain if if everyone else suddenly hated it.
  
  
  ok. Maybe it is just a question to get used to it. 
  also we'll see if we get user feedback.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, ngraham, hpereiradacosta
Cc: alake, colomar, januz, fabianr, mmustac, abetts, anemeth, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart


D10438: reserve space for checkable widgets in menu items

2018-02-13 Thread Nathaniel Graham
ngraham added a comment.


  FWIW I like it, but since this is mostly subjective, I wouldn't fight to the 
death to retain if if everyone else suddenly hated it.

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, ngraham, hpereiradacosta
Cc: alake, colomar, januz, fabianr, mmustac, abetts, anemeth, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart


D10438: reserve space for checkable widgets in menu items

2018-02-13 Thread Hugo Pereira Da Costa
hpereiradacosta added subscribers: colomar, alake.
hpereiradacosta added a comment.


  So ... I applied the patch (which I approved), used it, and ... don't like it 
sorry.
  See attached screenshots
  I think the unnecessary space breaks alignment with the menu above. and is 
completely unnecessary. Is this _really_ what we want ? @ngraham ? @alake ? 
@colomar
  
  F5709899: Screenshot_20180213_155307.png 

  
  F5709901: Screenshot_20180213_155334.png 


REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, ngraham, hpereiradacosta
Cc: alake, colomar, januz, fabianr, mmustac, abetts, anemeth, plasma-devel, 
ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, sebas, apol, mart


D10475: Make it possible to show the title despite having ctx actions

2018-02-13 Thread Marco Martin
mart added a comment.


  I like it!
  tough i think the more actions button should always be on the right of 
everything else, including the title

REPOSITORY
  R169 Kirigami

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

To: apol, #kirigami
Cc: mart, ngraham, plasma-devel, apol, davidedmundson, hein


D10482: Add an isNull() check before setting wheter QIcon is a mask

2018-02-13 Thread Igor Cota
icota created this revision.
icota added a reviewer: Kirigami.
icota added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.
icota requested review of this revision.

REVISION SUMMARY
  To avoid stepping on a null QIcon. This can happen when calling ::fromTheme 
with a icon name that doesn't exist on the platform

REPOSITORY
  R169 Kirigami

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

AFFECTED FILES
  src/libkirigami/platformtheme.cpp

To: icota, #kirigami
Cc: plasma-devel, apol, davidedmundson, mart, hein


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag added a comment.


  > this is because you have to rely on maxIconWidth, even if the current item 
has no icon, when computing the checkbox space.
  
  I've came up with something like this
  
if( showIcon && menuItemOption->maxIconWidth > 0 )
{
int dx = (iconRect.left() - Metrics::CheckBox_Size) / 2;
checkBoxRect.moveLeft( dx );
} else {
int dx = (textRect.left() - Metrics::CheckBox_Size) / 2;
checkBoxRect.moveLeft( dx );
}
  
  so, it centers properly checkboxes in menu items with and without icons
  
  F5709869: f.png 
  
  F5709874: g.png 
  
  //(it can be simplified with a ternary operator)//
  
  is it a good approach?

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10481: Add FormLayout.qml to kirigami.qrc

2018-02-13 Thread Igor Cota
icota created this revision.
icota added a reviewer: Kirigami.
icota added a project: Kirigami.
Restricted Application added a subscriber: plasma-devel.
icota requested review of this revision.

REVISION SUMMARY
  Can't use form layout in an app that includes(kirigami.pri) otherwise

REPOSITORY
  R169 Kirigami

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

AFFECTED FILES
  kirigami.qrc

To: icota, #kirigami
Cc: plasma-devel, apol, davidedmundson, mart, hein


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag marked 2 inline comments as done.
zzag added inline comments.

INLINE COMMENTS

> hpereiradacosta wrote in breezestyle.cpp:4701
> why was this chunk of code moved ? This is unrelated to the change.
> Please try keep the diff to the minimum

To keep it consistent:

1. compute required rectangles
2. align checkbox rect
3. paint text, arrows, etc.

Could you please let it stay here?

REPOSITORY
  R31 Breeze

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

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


D10480: align checkable widgets in menu items

2018-02-13 Thread Vlad Zagorodniy
zzag updated this revision to Diff 27062.
zzag added a comment.


  delete changes not related to this diff

REPOSITORY
  R31 Breeze

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10480?vs=27051=27062

BRANCH
  center-checkbox

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

AFFECTED FILES
  kstyle/breezestyle.cpp

To: zzag, #breeze, #vdg, hpereiradacosta
Cc: plasma-devel, ZrenBot, progwolff, lesliezhai, ali-mohamed, jensreuterberg, 
abetts, sebas, apol, mart


  1   2   >