Re: Review Request 120849: [tabbox] Try locating the WindowSwitcher QML through configured lnf package

2014-10-29 Thread Martin Gräßlin


 On Oct. 28, 2014, 11:15 p.m., Thomas Lübking wrote:
  tabbox/tabboxhandler.cpp, line 235
  https://git.reviewboard.kde.org/r/120849/diff/1/?file=322492#file322492line235
 
  why not? (by different name, oc)

yep, I was also considering moving the desktop switchers to lnf.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120849/#review69364
---


On Oct. 28, 2014, 4:53 p.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120849/
 ---
 
 (Updated Oct. 28, 2014, 4:53 p.m.)
 
 
 Review request for kwin and Plasma.
 
 
 Repository: kwin
 
 
 Description
 ---
 
 We assume the configured layout name is a look and feel package. Thus
 the Switcher is located at contents/windowswitcher/WindowSwitcher.qml
 of that package.
 
 
 Diffs
 -
 
   tabbox/tabboxconfig.h f953102986b2d5577cc89321e7fecdea336bfc07 
   tabbox/tabboxhandler.cpp cae64e3aec31af223a51fa4126798c736df411f1 
 
 Diff: https://git.reviewboard.kde.org/r/120849/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120847: [kcm/lookandfeel] Add support for changing WindowSwitcher

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120847/
---

(Updated Oct. 29, 2014, 7:26 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-desktop


Description
---

Configuration for kwinrc and reloading KWin.

As I'm not quite sure how the mechanism works, please verify that everything 
needed is loaded, saved, set, whatever :-)


Diffs
-

  kcms/lookandfeel/autotests/kcmtest.cpp 
08fae140be58fc3fe105b5d8770c764f077359e5 
  kcms/lookandfeel/autotests/lookandfeel/contents/defaults 
d16963a33a88ada2d41d966bef64a972474064bc 
  kcms/lookandfeel/kcm.h b6b6ec99884814037ea53ca8692ed9d19a22811b 
  kcms/lookandfeel/kcm.cpp 3ede2e05e339fbea17c504d631ccaea90728c74b 

Diff: https://git.reviewboard.kde.org/r/120847/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120850: Move KWin's window switcher sidebar to Look'n'Feel package

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120850/
---

(Updated Oct. 29, 2014, 7:27 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Repository: plasma-workspace


Description
---

Move KWin's window switcher sidebar to Look'n'Feel package


Diffs
-

  lookandfeel/contents/defaults 1dfe182e305f0a4ca2e0daad56a581efd68b930b 
  lookandfeel/contents/windowswitcher/WindowSwitcher.qml 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 

Diff: https://git.reviewboard.kde.org/r/120850/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120849: [tabbox] Try locating the WindowSwitcher QML through configured lnf package

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120849/
---

(Updated Oct. 29, 2014, 7:30 a.m.)


Status
--

This change has been marked as submitted.


Review request for kwin and Plasma.


Repository: kwin


Description
---

We assume the configured layout name is a look and feel package. Thus
the Switcher is located at contents/windowswitcher/WindowSwitcher.qml
of that package.


Diffs
-

  tabbox/tabboxconfig.h f953102986b2d5577cc89321e7fecdea336bfc07 
  tabbox/tabboxhandler.cpp cae64e3aec31af223a51fa4126798c736df411f1 

Diff: https://git.reviewboard.kde.org/r/120849/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120880: Import window switchers from KWin repository

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120880/
---

Review request for kwin and Plasma.


Repository: kdeplasma-addons


Description
---

It was decided that the default window switcher goes into the
look and feel package and all other window switchers go into
kdeplasma-addons. This means KWin does no longer offer any window
switchers.


Diffs
-

  windowswitchers/small_icons/contents/ui/main.qml PRE-CREATION 
  windowswitchers/small_icons/metadata.desktop PRE-CREATION 
  windowswitchers/text/contents/ui/main.qml PRE-CREATION 
  windowswitchers/text/metadata.desktop PRE-CREATION 
  windowswitchers/thumbnails/contents/ui/main.qml PRE-CREATION 
  windowswitchers/thumbnails/metadata.desktop PRE-CREATION 
  windowswitchers/present_windows/contents/ui/main.qml PRE-CREATION 
  windowswitchers/present_windows/metadata.desktop PRE-CREATION 
  windowswitchers/informative/metadata.desktop PRE-CREATION 
  windowswitchers/compact/metadata.desktop PRE-CREATION 
  windowswitchers/informative/contents/ui/main.qml PRE-CREATION 
  windowswitchers/big_icons/metadata.desktop PRE-CREATION 
  windowswitchers/compact/contents/ui/main.qml PRE-CREATION 
  windowswitchers/big_icons/contents/ui/main.qml PRE-CREATION 
  CMakeLists.txt 09180ea05aa4dad12a1788c72abc082707cf08af 
  windowswitchers/CMakeLists.txt PRE-CREATION 
  windowswitchers/IconTabBox.qml PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/120880/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120881: [tabbox] Window Switcher layouts moved to kdeplasma-addons

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120881/
---

Review request for kwin and Plasma.


Repository: kwin


Description
---

[tabbox] Window Switcher layouts moved to kdeplasma-addons


Diffs
-

  tabbox/qml/CMakeLists.txt e7d3b227eabf27622254ad8e50e97e6fcbec5c51 
  tabbox/qml/IconTabBox.qml 589d3da7e2f7eb7c27912687c833c1c239c8b1ff 
  tabbox/qml/clients/big_icons/contents/ui/main.qml 
74cef21ae0c782b432752477f3715994c73e459a 
  tabbox/qml/clients/big_icons/metadata.desktop 
1d13ad71d29b3a12161ffe023a2dbc6af69dbae8 
  tabbox/qml/clients/compact/contents/ui/main.qml 
c90959d62788e07f7da49782023f5df285d50988 
  tabbox/qml/clients/compact/metadata.desktop 
c96bfec03de451acdc9f4365c31bfb402851fcd4 
  tabbox/qml/clients/informative/contents/ui/main.qml 
e50dee6b4438eccee44129285be9ee61833cc15a 
  tabbox/qml/clients/informative/metadata.desktop 
6c8bd55e17aa7b5b5d792d11cab27dde0686efbf 
  tabbox/qml/clients/present_windows/contents/ui/main.qml 
d9cd3a0168970f59f1efafe278f8c0a763ba42db 
  tabbox/qml/clients/present_windows/metadata.desktop 
298e5e0d5b135e70d8102feeca131bba341a73ea 
  tabbox/qml/clients/small_icons/contents/ui/main.qml 
ec4a4d769e01c8f37db4af01911724f61934e496 
  tabbox/qml/clients/small_icons/metadata.desktop 
01d5d74e0a1b2138ca2c3ad2c2f4483c51af40fa 
  tabbox/qml/clients/text/contents/ui/main.qml 
79f97c9559062b7bebab4311d97deb4a1c8ea17b 
  tabbox/qml/clients/text/metadata.desktop 
a0752b597edf6b18b8a985bff035240cbdeca06c 
  tabbox/qml/clients/thumbnails/contents/ui/main.qml 
f70b3fc634fdb04651d8b843c5c290922dcb2c02 
  tabbox/qml/clients/thumbnails/metadata.desktop 
7329bc8c47942c1841d63094286676de03ab7d6c 

Diff: https://git.reviewboard.kde.org/r/120881/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120880: Import window switchers from KWin repository

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120880/#review69381
---


related review request for KWin: https://git.reviewboard.kde.org/r/120881/

- Martin Gräßlin


On Oct. 29, 2014, 8:50 a.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120880/
 ---
 
 (Updated Oct. 29, 2014, 8:50 a.m.)
 
 
 Review request for kwin and Plasma.
 
 
 Repository: kdeplasma-addons
 
 
 Description
 ---
 
 It was decided that the default window switcher goes into the
 look and feel package and all other window switchers go into
 kdeplasma-addons. This means KWin does no longer offer any window
 switchers.
 
 
 Diffs
 -
 
   windowswitchers/small_icons/contents/ui/main.qml PRE-CREATION 
   windowswitchers/small_icons/metadata.desktop PRE-CREATION 
   windowswitchers/text/contents/ui/main.qml PRE-CREATION 
   windowswitchers/text/metadata.desktop PRE-CREATION 
   windowswitchers/thumbnails/contents/ui/main.qml PRE-CREATION 
   windowswitchers/thumbnails/metadata.desktop PRE-CREATION 
   windowswitchers/present_windows/contents/ui/main.qml PRE-CREATION 
   windowswitchers/present_windows/metadata.desktop PRE-CREATION 
   windowswitchers/informative/metadata.desktop PRE-CREATION 
   windowswitchers/compact/metadata.desktop PRE-CREATION 
   windowswitchers/informative/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/metadata.desktop PRE-CREATION 
   windowswitchers/compact/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/contents/ui/main.qml PRE-CREATION 
   CMakeLists.txt 09180ea05aa4dad12a1788c72abc082707cf08af 
   windowswitchers/CMakeLists.txt PRE-CREATION 
   windowswitchers/IconTabBox.qml PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120880/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120882: [kcm/lookandfeel] Add support for changing DesktopSwitcher

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120882/
---

Review request for Plasma.


Repository: plasma-desktop


Description
---

Configuration for kwinrc and reloading KWin.


Diffs
-

  kcms/lookandfeel/kcm.cpp 3021b8c3cf12673a2816d0107aa22cdafb143049 
  kcms/lookandfeel/kcm.h f717ae3c79a5bfd758871812be7b0d273205a405 
  kcms/lookandfeel/autotests/lookandfeel/contents/defaults 
ae409881680389e3633cc9f92796ab1b2cf56e94 
  kcms/lookandfeel/autotests/kcmtest.cpp 
222962890cbbd561b481a7d5c07386056699a73e 

Diff: https://git.reviewboard.kde.org/r/120882/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120883: Move default DesktopSwitcher from KWin to Look and Feel Package

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120883/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

Move default DesktopSwitcher from KWin to Look and Feel Package


Diffs
-

  lookandfeel/contents/defaults 48b9e84babeeb9196a5fb3b06585a2e27d32d518 
  lookandfeel/contents/desktopswitcher/DesktopSwitcher.qml 
e69de29bb2d1d6434b8b29ae775ad8c2e48c5391 

Diff: https://git.reviewboard.kde.org/r/120883/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120884: [tabbox] Get DesktopSwitcher from look and feel package

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120884/
---

Review request for kwin and Plasma.


Repository: kwin


Description
---

Like with WindowSwitcher the configured value might be for the
look and feel package and thus we should first try to locate it
from the look and feel package.

[tabbox/qml] Default desktop switcher moved to look and feel package


Diffs
-

  tabbox/qml/CMakeLists.txt e7d3b227eabf27622254ad8e50e97e6fcbec5c51 
  tabbox/qml/desktops/informative/contents/ui/main.qml 
54179e6b3d3c222351b1efb4e9ef276dc28df880 
  tabbox/qml/desktops/informative/metadata.desktop 
a1495d58bca63577e0d39d87193e5b44a3a7ee0e 
  tabbox/tabbox.cpp aea7e11c635059850eec05045e68f661f15e7938 
  tabbox/tabboxhandler.cpp 44bed90f87da33fc5404b3cd8099d49edafabb5c 

Diff: https://git.reviewboard.kde.org/r/120884/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120877: Convert switch statements to if/else due to MSVC limitation

2014-10-29 Thread Kai Uwe Broulik

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120877/#review69384
---


No opinion on the patch but please check your coding style.


src/plasma/private/framesvg_helpers.h
https://git.reviewboard.kde.org/r/120877/#comment48486

Always put the brace on the same line as the else if


- Kai Uwe Broulik


On Okt. 29, 2014, 2:09 vorm., Andrius da Costa Ribas wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120877/
 ---
 
 (Updated Okt. 29, 2014, 2:09 vorm.)
 
 
 Review request for kdewin and Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 MSVC is unable to recognise e.g. FrameSvg::TopBorder | FrameSvg::LeftBorder 
 as a compile-time const expression (error C2051: case expression not constant)
 
 
 Diffs
 -
 
   src/plasma/private/framesvg_helpers.h 5b96cd5 
 
 Diff: https://git.reviewboard.kde.org/r/120877/diff/
 
 
 Testing
 ---
 
 Builds with MSVC 2013 64bit.
 
 framesvg related tests pass, other unrelated tests still fail.
 
 Test project V:/build/frameworks/plasma/work/msvc2013-RelWithDebInfo-master
   Start  1: plasma-dialogqmltest
  1/10 Test  #1: plasma-dialogqmltest ...***Failed1.15 sec
   Start  2: plasma-fallbackpackagetest
  2/10 Test  #2: plasma-fallbackpackagetest .   Passed0.16 sec
   Start  3: plasma-packagestructuretest
  3/10 Test  #3: plasma-packagestructuretest ***Failed0.19 sec
   Start  4: plasma-packageurlinterceptortest
  4/10 Test  #4: plasma-packageurlinterceptortest ...   Passed0.12 sec
   Start  5: plasma-pluginloadertest
  5/10 Test  #5: plasma-pluginloadertest    Passed1.44 sec
   Start  6: plasma-framesvgtest
  6/10 Test  #6: plasma-framesvgtest    Passed0.64 sec
   Start  7: coronatest
  7/10 Test  #7: coronatest .***Failed0.16 sec
   Start  8: plasma-storagetest
  8/10 Test  #8: plasma-storagetest .***Failed0.12 sec
   Start  9: plasma-sortfiltermodeltest
  9/10 Test  #9: plasma-sortfiltermodeltest .   Passed0.14 sec
   Start 10: i18ndcheck
 Could not find executable SH-NOTFOUND
 Looked in the following places:
 SH-NOTFOUND
 SH-NOTFOUND.exe
 Release/SH-NOTFOUND
 Release/SH-NOTFOUND.exe
 Debug/SH-NOTFOUND
 Debug/SH-NOTFOUND.exe
 MinSizeRel/SH-NOTFOUND
 MinSizeRel/SH-NOTFOUND.exe
 RelWithDebInfo/SH-NOTFOUND
 RelWithDebInfo/SH-NOTFOUND.exe
 Deployment/SH-NOTFOUND
 Deployment/SH-NOTFOUND.exe
 Development/SH-NOTFOUND
 Development/SH-NOTFOUND.exe
 Unable to find executable: SH-NOTFOUND
 10/10 Test #10: i18ndcheck .***Not Run   0.00 sec
 
 50% tests passed, 5 tests failed out of 10
 
 
 Thanks,
 
 Andrius da Costa Ribas
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/
---

Review request for Plasma.


Repository: plasma-framework


Description
---

Redone here, that seems a bit more understandable than gerrit.

It introduces a new status for applets, AwaitingDeletionStatus. triggering the 
delete action, puts the applet in AwaitingDeletionStatus. triggering it again 
it really deletes it. A notification with an undo action is emitted when the 
applet goes in AwaitingDeletionStatus.

The appelt is really deleted when either:
* A minute timeout expires
* The user manually closes the notification
* Plasma is shut down and the applet is in awaitingdeletion status

It would then be job for the qml part to actually hide applets that are 
AwaitingDeletionStatus as they don't exist anymore.


Diffs
-

  src/plasma/plasma.h 15c346b 
  src/plasma/private/applet_p.h 76a1270 
  src/plasma/private/applet_p.cpp 44ecd25 
  src/plasma/private/containment_p.cpp 3836772 
  src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
  src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
  CMakeLists.txt 10c0ef4 
  src/plasma/CMakeLists.txt 7cc2fe3 
  src/plasma/applet.cpp f4b5410 
  src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/120885/diff/


Testing
---

I'm not 100% sold on the technical approach on how is done, but it seems to 
work reliably


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Kai Uwe Broulik

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69387
---



src/plasma/private/applet_p.cpp
https://git.reviewboard.kde.org/r/120885/#comment48489

The notification should have the plasmoid icon, no?



src/plasma/private/applet_p.cpp
https://git.reviewboard.kde.org/r/120885/#comment48492

Does removing notifications actually work with our current notification 
plasmoid?


- Kai Uwe Broulik


On Okt. 29, 2014, 9:28 vorm., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Okt. 29, 2014, 9:28 vorm.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Martin Klapetek

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69385
---



src/plasma/data/notifications/plasmashell.notifyrc
https://git.reviewboard.kde.org/r/120885/#comment48487

You myyybe also want Sound?



src/plasma/private/applet_p.cpp
https://git.reviewboard.kde.org/r/120885/#comment48490

Can you confirm that after the timeout timeouts, the notification is 
removed from the notification history popup?

Just wondering if it actually removes it on close()...



src/plasma/private/applet_p.cpp
https://git.reviewboard.kde.org/r/120885/#comment48491

I /think/ removed is better fitting than deleted here?


- Martin Klapetek


On Oct. 29, 2014, 10:28 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 10:28 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Marco Martin


 On Oct. 29, 2014, 9:39 a.m., Kai Uwe Broulik wrote:
  src/plasma/private/applet_p.cpp, line 240
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322833#file322833line240
 
  The notification should have the plasmoid icon, no?

indeed. knotification doesn't have api for just icons, just possible to set a 
qpixmap.
what i'm concerned is that taking the icon pixmap may end up with a bad scaling 
if the view has a different size


 On Oct. 29, 2014, 9:39 a.m., Kai Uwe Broulik wrote:
  src/plasma/private/applet_p.cpp, line 272
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322833#file322833line272
 
  Does removing notifications actually work with our current notification 
  plasmoid?

yep


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69387
---


On Oct. 29, 2014, 9:28 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 9:28 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Marco Martin


 On Oct. 29, 2014, 9:39 a.m., Martin Klapetek wrote:
  src/plasma/data/notifications/plasmashell.notifyrc, line 9
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322830#file322830line9
 
  You myyybe also want Sound?

hmm, really?


 On Oct. 29, 2014, 9:39 a.m., Martin Klapetek wrote:
  src/plasma/private/applet_p.cpp, line 238
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322833#file322833line238
 
  Can you confirm that after the timeout timeouts, the notification is 
  removed from the notification history popup?
  
  Just wondering if it actually removes it on close()...

yep, the notification is going away from the history at timeout


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69385
---


On Oct. 29, 2014, 9:28 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 9:28 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69388
---


out of interest: what's the bool transient about?


src/plasma/data/notifications/plasmashell.notifyrc
https://git.reviewboard.kde.org/r/120885/#comment48496

Isn't Plasmoid a little bit too techy?



src/plasma/plasma.h
https://git.reviewboard.kde.org/r/120885/#comment48493

add a @since 5.x, with x whatever we currently are ;-)



src/plasma/private/applet_p.cpp
https://git.reviewboard.kde.org/r/120885/#comment48498

Q_NULLPTR



src/plasma/private/applet_p.cpp
https://git.reviewboard.kde.org/r/120885/#comment48499

not sure whether it's allowed in frameworks:

const QStringList actions({i18n(Undo)});


- Martin Gräßlin


On Oct. 29, 2014, 10:28 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 10:28 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Martin Gräßlin


 On Oct. 29, 2014, 10:39 a.m., Martin Klapetek wrote:
  src/plasma/data/notifications/plasmashell.notifyrc, line 9
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322830#file322830line9
 
  You myyybe also want Sound?
 
 Marco Martin wrote:
 hmm, really?

out of interest: what do you think would happen if a user has a Plasmoid with 
QtMultimedia after our yesterday's interesting gstreamer fun?


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69385
---


On Oct. 29, 2014, 10:28 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 10:28 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Martin Gräßlin


 On Oct. 29, 2014, 10:39 a.m., Martin Klapetek wrote:
  src/plasma/data/notifications/plasmashell.notifyrc, line 9
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322830#file322830line9
 
  You myyybe also want Sound?
 
 Marco Martin wrote:
 hmm, really?
 
 Martin Gräßlin wrote:
 out of interest: what do you think would happen if a user has a Plasmoid 
 with QtMultimedia after our yesterday's interesting gstreamer fun?

 hmm, really?

you remember the broken glass sound from 3.x? That would be perrrfect :-P


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69385
---


On Oct. 29, 2014, 10:28 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 10:28 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120884: [tabbox] Get DesktopSwitcher from look and feel package

2014-10-29 Thread Thomas Lübking

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120884/#review69397
---

Ship it!


since the branches only differ by a substring which is also the same in the 
service lookup, it might be reasonable to only branch that string on top of 
action?

- Thomas Lübking


On Okt. 29, 2014, 8:55 vorm., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120884/
 ---
 
 (Updated Okt. 29, 2014, 8:55 vorm.)
 
 
 Review request for kwin and Plasma.
 
 
 Repository: kwin
 
 
 Description
 ---
 
 Like with WindowSwitcher the configured value might be for the
 look and feel package and thus we should first try to locate it
 from the look and feel package.
 
 [tabbox/qml] Default desktop switcher moved to look and feel package
 
 
 Diffs
 -
 
   tabbox/qml/CMakeLists.txt e7d3b227eabf27622254ad8e50e97e6fcbec5c51 
   tabbox/qml/desktops/informative/contents/ui/main.qml 
 54179e6b3d3c222351b1efb4e9ef276dc28df880 
   tabbox/qml/desktops/informative/metadata.desktop 
 a1495d58bca63577e0d39d87193e5b44a3a7ee0e 
   tabbox/tabbox.cpp aea7e11c635059850eec05045e68f661f15e7938 
   tabbox/tabboxhandler.cpp 44bed90f87da33fc5404b3cd8099d49edafabb5c 
 
 Diff: https://git.reviewboard.kde.org/r/120884/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/
---

(Updated Oct. 29, 2014, 10:16 a.m.)


Review request for Plasma.


Repository: plasma-framework


Description
---

Redone here, that seems a bit more understandable than gerrit.

It introduces a new status for applets, AwaitingDeletionStatus. triggering the 
delete action, puts the applet in AwaitingDeletionStatus. triggering it again 
it really deletes it. A notification with an undo action is emitted when the 
applet goes in AwaitingDeletionStatus.

The appelt is really deleted when either:
* A minute timeout expires
* The user manually closes the notification
* Plasma is shut down and the applet is in awaitingdeletion status

It would then be job for the qml part to actually hide applets that are 
AwaitingDeletionStatus as they don't exist anymore.


Diffs (updated)
-

  src/plasma/plasma.h 15c346b 
  src/plasma/private/applet_p.h 76a1270 
  src/plasma/private/applet_p.cpp 44ecd25 
  src/plasma/private/containment_p.cpp 3836772 
  src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
  CMakeLists.txt 10c0ef4 
  src/plasma/CMakeLists.txt 7cc2fe3 
  src/plasma/applet.cpp f4b5410 
  src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
  src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 

Diff: https://git.reviewboard.kde.org/r/120885/diff/


Testing
---

I'm not 100% sold on the technical approach on how is done, but it seems to 
work reliably


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Marco Martin


 On Oct. 29, 2014, 9:49 a.m., Martin Gräßlin wrote:
  src/plasma/private/applet_p.cpp, lines 239-241
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322833#file322833line239
 
  not sure whether it's allowed in frameworks:
  
  const QStringList actions({i18n(Undo)});

not sure, but the old stupid way fine for me?


 On Oct. 29, 2014, 9:49 a.m., Martin Gräßlin wrote:
  src/plasma/data/notifications/plasmashell.notifyrc, lines 7-8
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322830#file322830line7
 
  Isn't Plasmoid a little bit too techy?

changed (even tough that text never gets actually used)


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69388
---


On Oct. 29, 2014, 9:28 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 9:28 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Martin Gräßlin


 On Oct. 29, 2014, 10:49 a.m., Martin Gräßlin wrote:
  src/plasma/data/notifications/plasmashell.notifyrc, lines 7-8
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322830#file322830line7
 
  Isn't Plasmoid a little bit too techy?
 
 Marco Martin wrote:
 changed (even tough that text never gets actually used)

I think that would be shown in the notifications KCM


 On Oct. 29, 2014, 10:49 a.m., Martin Gräßlin wrote:
  src/plasma/private/applet_p.cpp, lines 239-241
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322833#file322833line239
 
  not sure whether it's allowed in frameworks:
  
  const QStringList actions({i18n(Undo)});
 
 Marco Martin wrote:
 not sure, but the old stupid way fine for me?

yeah sure, it's just that the compiler can generate the list in a slightly more 
efficient way.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69388
---


On Oct. 29, 2014, 11:16 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 11:16 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120884: [tabbox] Get DesktopSwitcher from look and feel package

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120884/
---

(Updated Oct. 29, 2014, 11:26 a.m.)


Review request for kwin and Plasma.


Changes
---

non branching variant


Repository: kwin


Description (updated)
---

Like with WindowSwitcher the configured value might be for the
look and feel package and thus we should first try to locate it
from the look and feel package.

[tabbox/qml] Default desktop switcher moved to look and feel package


Diffs (updated)
-

  tabbox/tabbox.cpp aea7e11c635059850eec05045e68f661f15e7938 
  tabbox/tabboxhandler.cpp 44bed90f87da33fc5404b3cd8099d49edafabb5c 
  tabbox/qml/CMakeLists.txt e7d3b227eabf27622254ad8e50e97e6fcbec5c51 
  tabbox/qml/desktops/informative/contents/ui/main.qml 
54179e6b3d3c222351b1efb4e9ef276dc28df880 
  tabbox/qml/desktops/informative/metadata.desktop 
a1495d58bca63577e0d39d87193e5b44a3a7ee0e 

Diff: https://git.reviewboard.kde.org/r/120884/diff/


Testing
---


Thanks,

Martin Gräßlin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Andre Heinecke

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/
---

(Updated Oct. 29, 2014, 11:38 a.m.)


Review request for kwin, Plasma and Martin Gräßlin.


Changes
---

Added plasma as reviewers.


Bugs: 292566
http://bugs.kde.org/show_bug.cgi?id=292566


Repository: kde-workspace


Description
---

I'm reopening this review request as I have updated this Window Switcher for 
Plasma 5.1 and would like to get another review to check if there are any 
suggestions or issues regarding the port to the new API.

Secondly I would like to ask again to have this Window Switcher Layout included 
in the KWin repository. I would prefer it if users could obtain this layout 
from their trusted distributors and did not have to rely on an unverifyable 
third party download to obtain this plugin. 

As suggested in the original review I've put this up on kde-look and recieved 
some positive feedback. But I really feel that it is rotting away there and 
that KDE-Look is not a good place to distribute executable plugins.

IMHO the approach of this Window Switcher is different enough from the others 
included in KWin to be a useful addition to the fold. Especially as this 
behavior is already familiar to KDE users from some versions  4.6

It should also be close enough to the other layouts like thumbnails to keep 
maintenance very similar (I've mostly looked at the changes made to thumbnails 
to adapt this for Plasma 5)


Original description:

This Layout tries to mimic some of the old KDE 4.6 tabbox behavior and layout, 
it scales the thumbnails shown in the tabbox to avoid scrolling.
There are also three different states in this layout depending on the size of 
the scaled thumbnails to provide appropriate information even when there are 
many opened windows.

States:
1. Thumbnails are larger then 200px: Show the Title and the Icon of the Window 
directly below the thumbnail.
2. Thumbnails are between 200px and 64px: Thumbnails are shown together with 
the icon but only the title of the currently selected window is shown.
3. Thumbnails would be smaller then 64px: Only the Icons of the windows are 
shown and the title of the currently selected window (like big icons)
If the Thumbnails would be smaller then 32px the tabbox starts to scroll in the 
Icon Only state.

Size of the thumbnails depends on the screen size and the number of opened 
windows.


Diffs
-

  tabbox/qml/CMakeLists.txt fc55ab9 
  tabbox/qml/clients/scaling/contents/ui/main.qml PRE-CREATION 
  tabbox/qml/clients/scaling/metadata.desktop PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/109832/diff/


Testing
---

Tested with plasma 5.3.1 from Kubuntu next / unstable repositories.


Thanks,

Andre Heinecke

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69421
---


I tried to do it in a single squashed commit again on gerrit:
https://gerrit.vesnicky.cesnet.cz/r/#/c/131/

- Marco Martin


On Oct. 29, 2014, 10:16 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 10:16 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120884: [tabbox] Get DesktopSwitcher from look and feel package

2014-10-29 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120884/#review69423
---


+1

- Marco Martin


On Ott. 29, 2014, 10:26 a.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120884/
 ---
 
 (Updated Ott. 29, 2014, 10:26 a.m.)
 
 
 Review request for kwin and Plasma.
 
 
 Repository: kwin
 
 
 Description
 ---
 
 Like with WindowSwitcher the configured value might be for the
 look and feel package and thus we should first try to locate it
 from the look and feel package.
 
 [tabbox/qml] Default desktop switcher moved to look and feel package
 
 
 Diffs
 -
 
   tabbox/tabbox.cpp aea7e11c635059850eec05045e68f661f15e7938 
   tabbox/tabboxhandler.cpp 44bed90f87da33fc5404b3cd8099d49edafabb5c 
   tabbox/qml/CMakeLists.txt e7d3b227eabf27622254ad8e50e97e6fcbec5c51 
   tabbox/qml/desktops/informative/contents/ui/main.qml 
 54179e6b3d3c222351b1efb4e9ef276dc28df880 
   tabbox/qml/desktops/informative/metadata.desktop 
 a1495d58bca63577e0d39d87193e5b44a3a7ee0e 
 
 Diff: https://git.reviewboard.kde.org/r/120884/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Kai Uwe Broulik

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/#review69422
---



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48537

Is this needed? Adds a bit of overhead and you're not using it consistently 
everywhere anyway



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48538

Don't hardcode sizes, use units.gridUnit et al

You also might want to make those properties readonly



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48539

Those don't seem t be used



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48540

Be careful with clipping, it's quite expensive



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48541

You don't need ; in qml definitions



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48542

Then use FrameSvg not FrameSvgItem



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48544

Is the lagging behind highlight really needed? :/



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48543

Margins only apply to corners where it's anchored to



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48545

Put id at the beginning of the item



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48546

Doesn't have a height



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48547

property variant is obsolete, use property var instead



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48548

spaces, foo * (1.0 / bar)



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48549

You don't need the {} when it's just one line



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48558

visible: false (space)



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48557

It's not anchored to the bottom, making the margin have no effect



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48550

Use units.iconSizes.xxx



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48556

Please check the order, it's all higgledy-piggledy



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48555

font.bold: true



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48551

Could this be done in a declarative, rather than imperative, way?



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48552

/**



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48554

This could be inlined in the label above
text: {
  the code
}



tabbox/qml/clients/scaling/contents/ui/main.qml
https://git.reviewboard.kde.org/r/109832/#comment48553

i18n this?


- Kai Uwe Broulik


On Okt. 29, 2014, 11:38 vorm., Andre Heinecke wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/109832/
 ---
 
 (Updated Okt. 29, 2014, 11:38 vorm.)
 
 
 Review request for kwin, Plasma and Martin Gräßlin.
 
 
 Bugs: 292566
 http://bugs.kde.org/show_bug.cgi?id=292566
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 I'm reopening this review request as I have updated this Window Switcher for 
 Plasma 5.1 and would like to get another review to check if there are any 
 suggestions or issues regarding the port to the new API.
 
 Secondly I would like to ask again to have this Window Switcher Layout 
 included in the KWin repository. I would prefer it if users could obtain this 
 layout from their trusted distributors and did not have to rely on an 
 unverifyable third party download to obtain this plugin. 
 
 As suggested in the original review I've put this up on kde-look and recieved 
 some positive feedback. But I really feel that it is rotting away there and 
 that KDE-Look is not a good place to distribute executable plugins.
 
 IMHO the approach of this Window Switcher is different enough from the others 
 included in 

Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Thomas Lübking

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/#review69429
---


The version check is actually on the bugzilla version (KDE_MAKE_VERSION is just 
a bitshifting macro) - it's the important part in the original patch ;-)

- Thomas Lübking


On Okt. 28, 2014, 11:02 nachm., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120876/
 ---
 
 (Updated Okt. 28, 2014, 11:02 nachm.)
 
 
 Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.
 
 
 Bugs: 337742
 https://bugs.kde.org/show_bug.cgi?id=337742
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 discussion was in https://git.reviewboard.kde.org/r/120431/
 removed the version checks, as we know we have kdelibs = 4.5 ;-)
 
 
 Diffs
 -
 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 
   drkonqi/bugzillaintegration/bugzillalib.h 570169b 
   drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
 
 Diff: https://git.reviewboard.kde.org/r/120876/diff/
 
 
 Testing
 ---
 
 builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
 before.
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Thomas Lübking


 On Okt. 29, 2014, 12:41 nachm., Thomas Lübking wrote:
  The version check is actually on the bugzilla version (KDE_MAKE_VERSION is 
  just a bitshifting macro) - it's the important part in the original patch 
  ;-)

To prevent future confusion, one might want to add

#define KDE_MAKE_BUGZILLA_VERSION KDE_MAKE_VERSION

or sth. like this.


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/#review69429
---


On Okt. 28, 2014, 11:02 nachm., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120876/
 ---
 
 (Updated Okt. 28, 2014, 11:02 nachm.)
 
 
 Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.
 
 
 Bugs: 337742
 https://bugs.kde.org/show_bug.cgi?id=337742
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 discussion was in https://git.reviewboard.kde.org/r/120431/
 removed the version checks, as we know we have kdelibs = 4.5 ;-)
 
 
 Diffs
 -
 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 
   drkonqi/bugzillaintegration/bugzillalib.h 570169b 
   drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
 
 Diff: https://git.reviewboard.kde.org/r/120876/diff/
 
 
 Testing
 ---
 
 builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
 before.
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120885: basic undo for plasmoids deletion

2014-10-29 Thread Martin Klapetek


 On Oct. 29, 2014, 10:39 a.m., Martin Klapetek wrote:
  src/plasma/data/notifications/plasmashell.notifyrc, line 9
  https://git.reviewboard.kde.org/r/120885/diff/1/?file=322830#file322830line9
 
  You myyybe also want Sound?
 
 Marco Martin wrote:
 hmm, really?
 
 Martin Gräßlin wrote:
 out of interest: what do you think would happen if a user has a Plasmoid 
 with QtMultimedia after our yesterday's interesting gstreamer fun?
 
 Martin Gräßlin wrote:
  hmm, really?
 
 you remember the broken glass sound from 3.x? That would be perrrfect :-P

 out of interest: what do you think would happen if a user has a Plasmoid with 
 QtMultimedia after our yesterday's interesting gstreamer fun?

Eh. Let's hope it won't break the timespace continuum :S


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120885/#review69385
---


On Oct. 29, 2014, 11:16 a.m., Marco Martin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120885/
 ---
 
 (Updated Oct. 29, 2014, 11:16 a.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-framework
 
 
 Description
 ---
 
 Redone here, that seems a bit more understandable than gerrit.
 
 It introduces a new status for applets, AwaitingDeletionStatus. triggering 
 the delete action, puts the applet in AwaitingDeletionStatus. triggering it 
 again it really deletes it. A notification with an undo action is emitted 
 when the applet goes in AwaitingDeletionStatus.
 
 The appelt is really deleted when either:
 * A minute timeout expires
 * The user manually closes the notification
 * Plasma is shut down and the applet is in awaitingdeletion status
 
 It would then be job for the qml part to actually hide applets that are 
 AwaitingDeletionStatus as they don't exist anymore.
 
 
 Diffs
 -
 
   src/plasma/plasma.h 15c346b 
   src/plasma/private/applet_p.h 76a1270 
   src/plasma/private/applet_p.cpp 44ecd25 
   src/plasma/private/containment_p.cpp 3836772 
   src/scriptengines/qml/plasmoid/appletinterface.cpp 24a36b3 
   CMakeLists.txt 10c0ef4 
   src/plasma/CMakeLists.txt 7cc2fe3 
   src/plasma/applet.cpp f4b5410 
   src/plasma/data/notifications/plasmashell.notifyrc PRE-CREATION 
   src/scriptengines/qml/plasmoid/containmentinterface.cpp fae64c6 
 
 Diff: https://git.reviewboard.kde.org/r/120885/diff/
 
 
 Testing
 ---
 
 I'm not 100% sold on the technical approach on how is done, but it seems to 
 work reliably
 
 
 Thanks,
 
 Marco Martin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120352: Remove keyboard click volume setting

2014-10-29 Thread Frederik Gladhorn

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120352/#review69438
---


I don't have much of an opinion when it comes to keyboard sounds. For virtual 
keybords this would certainly make sense, but this is about the hardware 
keyboard. If the feature never worked, then just remove it, it can always be 
added back and fixed when someone requests it.

- Frederik Gladhorn


On Sept. 28, 2014, 1:33 p.m., David Edmundson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120352/
 ---
 
 (Updated Sept. 28, 2014, 1:33 p.m.)
 
 
 Review request for Plasma and Frederik Gladhorn.
 
 
 Repository: plasma-desktop
 
 
 Description
 ---
 
 They keyboard daemon had a weird option for making your system make a
 short beep whenever you pressed a key.
 
 This doesn't work and doesn't seem particularly useful.
 
 
 Diffs
 -
 
   kcms/keyboard/kcmmisc.h 411bdd2 
   kcms/keyboard/kcmmisc.cpp c63d06b 
   kcms/keyboard/kcmmiscwidget.ui 37fbaf4 
   kcms/keyboard/keyboard_hardware.cpp df5f417 
 
 Diff: https://git.reviewboard.kde.org/r/120352/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David Edmundson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Andre Heinecke


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, lines 31-32
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line31
 
  Is this needed? Adds a bit of overhead and you're not using it 
  consistently everywhere anyway

Replaced through direct usage. Was an artifact taken over from thumbnails 
switcher.


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, lines 75-76
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line75
 
  Those don't seem t be used

Removed


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 79
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line79
 
  Be careful with clipping, it's quite expensive

Not sure why there is clipping here. I took this over from thumbnails layout 
and thought it would be there for a reason.
Removed now.


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 88
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line88
 
  You don't need ; in qml definitions

Removed.


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 132
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line132
 
  Then use FrameSvg not FrameSvgItem

Not sure what you mean here. Should I just take the margins from FrameSvg as 
the margins of the hover effect?
I understood this code to create a hover item to get the margins of that item. 
(Taken over from Thumbnails)


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 147
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line147
 
  Is the lagging behind highlight really needed? :/

I find 250 looks smooth enough. And it's the same as all other window tabbox 
plugins use so I would also prefer to use the same value here.
Longer and it gets sluggish. Shorter and it looks jumpy.


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, lines 153-155
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line153
 
  Margins only apply to corners where it's anchored to

Removed those margins.


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 162
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line162
 
  Put id at the beginning of the item

Done


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 163
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line163
 
  Doesn't have a height

The height is assigned in the states.
Is there an advantage to set one initially?


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, lines 167-168
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line167
 
  property variant is obsolete, use property var instead

changed


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 170
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line170
 
  spaces, foo * (1.0 / bar)

changed


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 36
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line36
 
  Don't hardcode sizes, use units.gridUnit et al
  
  You also might want to make those properties readonly

I've changed all hardcoded values (there were some hardcoded 5's where i meant 
icon spacing) to use properties and the basic size properties are now based on 
units.


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, lines 375-376
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line375
 
  Could this be done in a declarative, rather than imperative, way?

I use both functions more then once.
If i did it declartively I had to duplicate the code, right?


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 402
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line402
 
  This could be inlined in the label above
  text: {
the code
  }

Right. simplified this to: minimized ? ( + caption + ) : caption


 On Oct. 29, 2014, 11:57 a.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 405
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line405
 
  i18n this?

I don't think this is 

Re: Review Request 120880: Import window switchers from KWin repository

2014-10-29 Thread Thomas Pfeiffer

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120880/#review69457
---


That means that other desktop enviornments cannot really use KWin without 
installing at least one lf package anymore, right? Could that be a problem, or 
is KWin no fun without any LFs installed, anyway, even without that change?

- Thomas Pfeiffer


On Okt. 29, 2014, 7:50 vorm., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120880/
 ---
 
 (Updated Okt. 29, 2014, 7:50 vorm.)
 
 
 Review request for kwin and Plasma.
 
 
 Repository: kdeplasma-addons
 
 
 Description
 ---
 
 It was decided that the default window switcher goes into the
 look and feel package and all other window switchers go into
 kdeplasma-addons. This means KWin does no longer offer any window
 switchers.
 
 
 Diffs
 -
 
   windowswitchers/small_icons/contents/ui/main.qml PRE-CREATION 
   windowswitchers/small_icons/metadata.desktop PRE-CREATION 
   windowswitchers/text/contents/ui/main.qml PRE-CREATION 
   windowswitchers/text/metadata.desktop PRE-CREATION 
   windowswitchers/thumbnails/contents/ui/main.qml PRE-CREATION 
   windowswitchers/thumbnails/metadata.desktop PRE-CREATION 
   windowswitchers/present_windows/contents/ui/main.qml PRE-CREATION 
   windowswitchers/present_windows/metadata.desktop PRE-CREATION 
   windowswitchers/informative/metadata.desktop PRE-CREATION 
   windowswitchers/compact/metadata.desktop PRE-CREATION 
   windowswitchers/informative/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/metadata.desktop PRE-CREATION 
   windowswitchers/compact/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/contents/ui/main.qml PRE-CREATION 
   CMakeLists.txt 09180ea05aa4dad12a1788c72abc082707cf08af 
   windowswitchers/CMakeLists.txt PRE-CREATION 
   windowswitchers/IconTabBox.qml PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120880/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120880: Import window switchers from KWin repository

2014-10-29 Thread Martin Gräßlin


 On Oct. 29, 2014, 4:06 p.m., Thomas Pfeiffer wrote:
  That means that other desktop enviornments cannot really use KWin without 
  installing at least one lf package anymore, right? Could that be a 
  problem, or is KWin no fun without any LFs installed, anyway, even without 
  that change?

I already prepared a blog post and addressed the implication. Verbatim quote 
from the draft:

 The change has also some implications for users of non Plasma desktop 
 environments wanting to use KWin as their window manager. By moving the 
 switchers out, KWin removes some of the Plasma dependencies. All switchers 
 provided by KWin are using Plasma components, the default switcher is part of 
 the design concept for Plasma 5 following the same idea as other similar 
 components. Thus KWin had a direct dependency on Plasma with the window 
 switchers. This is now kind of solved by not offering any switcher at all.

 My suggestion for desktop environment projects wanting to use KWin is to 
 provide their own default Look'n'Feel package with a Window and Desktop 
 switcher specific for their environment. On the other hand I don't see a 
 problem with providing a simple fallback theme in KWin. And of course there 
 are still the Desktop Effects for switching between Windows (CoverSwitch and 
 FlipSwitch) installed by default and just need to be enabled.


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120880/#review69457
---


On Oct. 29, 2014, 8:50 a.m., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120880/
 ---
 
 (Updated Oct. 29, 2014, 8:50 a.m.)
 
 
 Review request for kwin and Plasma.
 
 
 Repository: kdeplasma-addons
 
 
 Description
 ---
 
 It was decided that the default window switcher goes into the
 look and feel package and all other window switchers go into
 kdeplasma-addons. This means KWin does no longer offer any window
 switchers.
 
 
 Diffs
 -
 
   windowswitchers/small_icons/contents/ui/main.qml PRE-CREATION 
   windowswitchers/small_icons/metadata.desktop PRE-CREATION 
   windowswitchers/text/contents/ui/main.qml PRE-CREATION 
   windowswitchers/text/metadata.desktop PRE-CREATION 
   windowswitchers/thumbnails/contents/ui/main.qml PRE-CREATION 
   windowswitchers/thumbnails/metadata.desktop PRE-CREATION 
   windowswitchers/present_windows/contents/ui/main.qml PRE-CREATION 
   windowswitchers/present_windows/metadata.desktop PRE-CREATION 
   windowswitchers/informative/metadata.desktop PRE-CREATION 
   windowswitchers/compact/metadata.desktop PRE-CREATION 
   windowswitchers/informative/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/metadata.desktop PRE-CREATION 
   windowswitchers/compact/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/contents/ui/main.qml PRE-CREATION 
   CMakeLists.txt 09180ea05aa4dad12a1788c72abc082707cf08af 
   windowswitchers/CMakeLists.txt PRE-CREATION 
   windowswitchers/IconTabBox.qml PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120880/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120880: Import window switchers from KWin repository

2014-10-29 Thread Thomas Pfeiffer


 On Okt. 29, 2014, 3:06 nachm., Thomas Pfeiffer wrote:
  That means that other desktop enviornments cannot really use KWin without 
  installing at least one lf package anymore, right? Could that be a 
  problem, or is KWin no fun without any LFs installed, anyway, even without 
  that change?
 
 Martin Gräßlin wrote:
 I already prepared a blog post and addressed the implication. Verbatim 
 quote from the draft:
 
  The change has also some implications for users of non Plasma desktop 
 environments wanting to use KWin as their window manager. By moving the 
 switchers out, KWin removes some of the Plasma dependencies. All switchers 
 provided by KWin are using Plasma components, the default switcher is part of 
 the design concept for Plasma 5 following the same idea as other similar 
 components. Thus KWin had a direct dependency on Plasma with the window 
 switchers. This is now kind of solved by not offering any switcher at all.
 
  My suggestion for desktop environment projects wanting to use KWin is 
 to provide their own default Look'n'Feel package with a Window and Desktop 
 switcher specific for their environment. On the other hand I don't see a 
 problem with providing a simple fallback theme in KWin. And of course there 
 are still the Desktop Effects for switching between Windows (CoverSwitch and 
 FlipSwitch) installed by default and just need to be enabled.

Excellent explanation! I rest my case.


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120880/#review69457
---


On Okt. 29, 2014, 7:50 vorm., Martin Gräßlin wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120880/
 ---
 
 (Updated Okt. 29, 2014, 7:50 vorm.)
 
 
 Review request for kwin and Plasma.
 
 
 Repository: kdeplasma-addons
 
 
 Description
 ---
 
 It was decided that the default window switcher goes into the
 look and feel package and all other window switchers go into
 kdeplasma-addons. This means KWin does no longer offer any window
 switchers.
 
 
 Diffs
 -
 
   windowswitchers/small_icons/contents/ui/main.qml PRE-CREATION 
   windowswitchers/small_icons/metadata.desktop PRE-CREATION 
   windowswitchers/text/contents/ui/main.qml PRE-CREATION 
   windowswitchers/text/metadata.desktop PRE-CREATION 
   windowswitchers/thumbnails/contents/ui/main.qml PRE-CREATION 
   windowswitchers/thumbnails/metadata.desktop PRE-CREATION 
   windowswitchers/present_windows/contents/ui/main.qml PRE-CREATION 
   windowswitchers/present_windows/metadata.desktop PRE-CREATION 
   windowswitchers/informative/metadata.desktop PRE-CREATION 
   windowswitchers/compact/metadata.desktop PRE-CREATION 
   windowswitchers/informative/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/metadata.desktop PRE-CREATION 
   windowswitchers/compact/contents/ui/main.qml PRE-CREATION 
   windowswitchers/big_icons/contents/ui/main.qml PRE-CREATION 
   CMakeLists.txt 09180ea05aa4dad12a1788c72abc082707cf08af 
   windowswitchers/CMakeLists.txt PRE-CREATION 
   windowswitchers/IconTabBox.qml PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/120880/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 Martin Gräßlin
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Andre Heinecke

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/
---

(Updated Oct. 29, 2014, 3:22 p.m.)


Review request for kwin, Plasma and Martin Gräßlin.


Changes
---

Updated patch


Bugs: 292566
http://bugs.kde.org/show_bug.cgi?id=292566


Repository: kde-workspace


Description
---

I'm reopening this review request as I have updated this Window Switcher for 
Plasma 5.1 and would like to get another review to check if there are any 
suggestions or issues regarding the port to the new API.

Secondly I would like to ask again to have this Window Switcher Layout included 
in the KWin repository. I would prefer it if users could obtain this layout 
from their trusted distributors and did not have to rely on an unverifyable 
third party download to obtain this plugin. 

As suggested in the original review I've put this up on kde-look and recieved 
some positive feedback. But I really feel that it is rotting away there and 
that KDE-Look is not a good place to distribute executable plugins.

IMHO the approach of this Window Switcher is different enough from the others 
included in KWin to be a useful addition to the fold. Especially as this 
behavior is already familiar to KDE users from some versions  4.6

It should also be close enough to the other layouts like thumbnails to keep 
maintenance very similar (I've mostly looked at the changes made to thumbnails 
to adapt this for Plasma 5)


Original description:

This Layout tries to mimic some of the old KDE 4.6 tabbox behavior and layout, 
it scales the thumbnails shown in the tabbox to avoid scrolling.
There are also three different states in this layout depending on the size of 
the scaled thumbnails to provide appropriate information even when there are 
many opened windows.

States:
1. Thumbnails are larger then 200px: Show the Title and the Icon of the Window 
directly below the thumbnail.
2. Thumbnails are between 200px and 64px: Thumbnails are shown together with 
the icon but only the title of the currently selected window is shown.
3. Thumbnails would be smaller then 64px: Only the Icons of the windows are 
shown and the title of the currently selected window (like big icons)
If the Thumbnails would be smaller then 32px the tabbox starts to scroll in the 
Icon Only state.

Size of the thumbnails depends on the screen size and the number of opened 
windows.


Diffs (updated)
-

  tabbox/qml/CMakeLists.txt fc55ab9 
  tabbox/qml/clients/scaling/contents/ui/main.qml PRE-CREATION 
  tabbox/qml/clients/scaling/metadata.desktop PRE-CREATION 

Diff: https://git.reviewboard.kde.org/r/109832/diff/


Testing
---

Tested with plasma 5.3.1 from Kubuntu next / unstable repositories.


Thanks,

Andre Heinecke

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Kai Uwe Broulik


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
 
 
 Martin Gräßlin wrote:
 @Kai Uwe: may I ask you to do such a review for all other existing window 
 switchers? Or just have a look at them and improve as you seem fit.

Okay, once they've all landed in kdeplasma-addons I will take a look.


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, lines 375-376
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line375
 
  Could this be done in a declarative, rather than imperative, way?
 
 Andre Heinecke wrote:
 I use both functions more then once.
 If i did it declartively I had to duplicate the code, right?

I was more thinking of something like

width: Math.min(textItem.contentWidth, scalingTabBox.width - 
captionFrame.anchors.leftMargin - captionFrame.anchors.rightMargin - 
captionIconItem.width * 2 - captionFrame.anchors.rightMargin)

instead of manually invoking that method whenever the width or text changes and 
let QML figure that out for us automagically.


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 405
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line405
 
  i18n this?
 
 Andre Heinecke wrote:
 I don't think this is neccessary. The braces are just used to indicate 
 that a window is minimized. They have no lingustic meaning in this context.

You cannot assume that a language uses exactly this kind of braces in that 
order:
i18nc(braces just denote the window is minimized, (%1), text)


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 163
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line163
 
  Doesn't have a height
 
 Andre Heinecke wrote:
 The height is assigned in the states.
 Is there an advantage to set one initially?

Sorry, I missed that. Fine then.


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 147
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line147
 
  Is the lagging behind highlight really needed? :/
 
 Andre Heinecke wrote:
 I find 250 looks smooth enough. And it's the same as all other window 
 tabbox plugins use so I would also prefer to use the same value here.
 Longer and it gets sluggish. Shorter and it looks jumpy.

Right, we need to ask VDG/HIG about that later then.


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 132
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line132
 
  Then use FrameSvg not FrameSvgItem
 
 Andre Heinecke wrote:
 Not sure what you mean here. Should I just take the margins from FrameSvg 
 as the margins of the hover effect?
 I understood this code to create a hover item to get the margins of that 
 item. (Taken over from Thumbnails)

I just forwarded that comment from David who complained about that in a 
different RR :)


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 79
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line79
 
  Be careful with clipping, it's quite expensive
 
 Andre Heinecke wrote:
 Not sure why there is clipping here. I took this over from thumbnails 
 layout and thought it would be there for a reason.
 Removed now.

Fair enough. Needs investigation/fixing in the others as well, Martin G said it 
might be needed by KWin internally.


 On Okt. 29, 2014, 11:57 vorm., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 36
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line36
 
  Don't hardcode sizes, use units.gridUnit et al
  
  You also might want to make those properties readonly
 
 Andre Heinecke wrote:
 I've changed all hardcoded values (there were some hardcoded 5's where i 
 meant icon spacing) to use properties and the basic size properties are now 
 based on units.

5 could be units.smallSpacing or so I guess


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/#review69422
---


On Okt. 29, 2014, 3:22 nachm., Andre Heinecke wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/109832/
 ---
 
 (Updated Okt. 29, 2014, 3:22 nachm.)
 
 
 Review request for kwin, Plasma and Martin Gräßlin.
 
 
 Bugs: 292566
 http://bugs.kde.org/show_bug.cgi?id=292566
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 I'm reopening 

Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Martin Klapetek


 On Oct. 29, 2014, 12:57 p.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 405
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line405
 
  i18n this?
 
 Andre Heinecke wrote:
 I don't think this is neccessary. The braces are just used to indicate 
 that a window is minimized. They have no lingustic meaning in this context.
 
 Kai Uwe Broulik wrote:
 You cannot assume that a language uses exactly this kind of braces in 
 that order:
 i18nc(braces just denote the window is minimized, (%1), text)

It's not about linguistic meaning, it's more about if those ()s will make sense 
in eg. Japaneese language or Arabic.

Simple rule to remember: if the string is going to appear anywhere on the 
screen, the whole string must be in i18n ;)


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/#review69422
---


On Oct. 29, 2014, 4:22 p.m., Andre Heinecke wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/109832/
 ---
 
 (Updated Oct. 29, 2014, 4:22 p.m.)
 
 
 Review request for kwin, Plasma and Martin Gräßlin.
 
 
 Bugs: 292566
 http://bugs.kde.org/show_bug.cgi?id=292566
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 I'm reopening this review request as I have updated this Window Switcher for 
 Plasma 5.1 and would like to get another review to check if there are any 
 suggestions or issues regarding the port to the new API.
 
 Secondly I would like to ask again to have this Window Switcher Layout 
 included in the KWin repository. I would prefer it if users could obtain this 
 layout from their trusted distributors and did not have to rely on an 
 unverifyable third party download to obtain this plugin. 
 
 As suggested in the original review I've put this up on kde-look and recieved 
 some positive feedback. But I really feel that it is rotting away there and 
 that KDE-Look is not a good place to distribute executable plugins.
 
 IMHO the approach of this Window Switcher is different enough from the others 
 included in KWin to be a useful addition to the fold. Especially as this 
 behavior is already familiar to KDE users from some versions  4.6
 
 It should also be close enough to the other layouts like thumbnails to keep 
 maintenance very similar (I've mostly looked at the changes made to 
 thumbnails to adapt this for Plasma 5)
 
 
 Original description:
 
 This Layout tries to mimic some of the old KDE 4.6 tabbox behavior and 
 layout, it scales the thumbnails shown in the tabbox to avoid scrolling.
 There are also three different states in this layout depending on the size of 
 the scaled thumbnails to provide appropriate information even when there are 
 many opened windows.
 
 States:
 1. Thumbnails are larger then 200px: Show the Title and the Icon of the 
 Window directly below the thumbnail.
 2. Thumbnails are between 200px and 64px: Thumbnails are shown together with 
 the icon but only the title of the currently selected window is shown.
 3. Thumbnails would be smaller then 64px: Only the Icons of the windows are 
 shown and the title of the currently selected window (like big icons)
 If the Thumbnails would be smaller then 32px the tabbox starts to scroll in 
 the Icon Only state.
 
 Size of the thumbnails depends on the screen size and the number of opened 
 windows.
 
 
 Diffs
 -
 
   tabbox/qml/CMakeLists.txt fc55ab9 
   tabbox/qml/clients/scaling/contents/ui/main.qml PRE-CREATION 
   tabbox/qml/clients/scaling/metadata.desktop PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/109832/diff/
 
 
 Testing
 ---
 
 Tested with plasma 5.3.1 from Kubuntu next / unstable repositories.
 
 
 Thanks,
 
 Andre Heinecke
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Martin Gräßlin


 On Oct. 29, 2014, 12:57 p.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 405
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line405
 
  i18n this?
 
 Andre Heinecke wrote:
 I don't think this is neccessary. The braces are just used to indicate 
 that a window is minimized. They have no lingustic meaning in this context.
 
 Kai Uwe Broulik wrote:
 You cannot assume that a language uses exactly this kind of braces in 
 that order:
 i18nc(braces just denote the window is minimized, (%1), text)
 
 Martin Klapetek wrote:
 It's not about linguistic meaning, it's more about if those ()s will make 
 sense in eg. Japaneese language or Arabic.
 
 Simple rule to remember: if the string is going to appear anywhere on 
 the screen, the whole string must be in i18n ;)

the usage of ()s for minimized windows has been used in KWin without i18n since 
at least as long as I'm involved in the project. I can look up when it got 
introduced and I would not be surprised if it's since KDE 2.0. Given that and 
no translators ever complained, I wouldn't change it ;-)


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/#review69422
---


On Oct. 29, 2014, 4:22 p.m., Andre Heinecke wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/109832/
 ---
 
 (Updated Oct. 29, 2014, 4:22 p.m.)
 
 
 Review request for kwin, Plasma and Martin Gräßlin.
 
 
 Bugs: 292566
 http://bugs.kde.org/show_bug.cgi?id=292566
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 I'm reopening this review request as I have updated this Window Switcher for 
 Plasma 5.1 and would like to get another review to check if there are any 
 suggestions or issues regarding the port to the new API.
 
 Secondly I would like to ask again to have this Window Switcher Layout 
 included in the KWin repository. I would prefer it if users could obtain this 
 layout from their trusted distributors and did not have to rely on an 
 unverifyable third party download to obtain this plugin. 
 
 As suggested in the original review I've put this up on kde-look and recieved 
 some positive feedback. But I really feel that it is rotting away there and 
 that KDE-Look is not a good place to distribute executable plugins.
 
 IMHO the approach of this Window Switcher is different enough from the others 
 included in KWin to be a useful addition to the fold. Especially as this 
 behavior is already familiar to KDE users from some versions  4.6
 
 It should also be close enough to the other layouts like thumbnails to keep 
 maintenance very similar (I've mostly looked at the changes made to 
 thumbnails to adapt this for Plasma 5)
 
 
 Original description:
 
 This Layout tries to mimic some of the old KDE 4.6 tabbox behavior and 
 layout, it scales the thumbnails shown in the tabbox to avoid scrolling.
 There are also three different states in this layout depending on the size of 
 the scaled thumbnails to provide appropriate information even when there are 
 many opened windows.
 
 States:
 1. Thumbnails are larger then 200px: Show the Title and the Icon of the 
 Window directly below the thumbnail.
 2. Thumbnails are between 200px and 64px: Thumbnails are shown together with 
 the icon but only the title of the currently selected window is shown.
 3. Thumbnails would be smaller then 64px: Only the Icons of the windows are 
 shown and the title of the currently selected window (like big icons)
 If the Thumbnails would be smaller then 32px the tabbox starts to scroll in 
 the Icon Only state.
 
 Size of the thumbnails depends on the screen size and the number of opened 
 windows.
 
 
 Diffs
 -
 
   tabbox/qml/CMakeLists.txt fc55ab9 
   tabbox/qml/clients/scaling/contents/ui/main.qml PRE-CREATION 
   tabbox/qml/clients/scaling/metadata.desktop PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/109832/diff/
 
 
 Testing
 ---
 
 Tested with plasma 5.3.1 from Kubuntu next / unstable repositories.
 
 
 Thanks,
 
 Andre Heinecke
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 109832: New tabbox layout with scaling thumbnails

2014-10-29 Thread Martin Gräßlin


 On Oct. 29, 2014, 12:57 p.m., Kai Uwe Broulik wrote:
  tabbox/qml/clients/scaling/contents/ui/main.qml, line 405
  https://git.reviewboard.kde.org/r/109832/diff/3/?file=321190#file321190line405
 
  i18n this?
 
 Andre Heinecke wrote:
 I don't think this is neccessary. The braces are just used to indicate 
 that a window is minimized. They have no lingustic meaning in this context.
 
 Kai Uwe Broulik wrote:
 You cannot assume that a language uses exactly this kind of braces in 
 that order:
 i18nc(braces just denote the window is minimized, (%1), text)
 
 Martin Klapetek wrote:
 It's not about linguistic meaning, it's more about if those ()s will make 
 sense in eg. Japaneese language or Arabic.
 
 Simple rule to remember: if the string is going to appear anywhere on 
 the screen, the whole string must be in i18n ;)
 
 Martin Gräßlin wrote:
 the usage of ()s for minimized windows has been used in KWin without i18n 
 since at least as long as I'm involved in the project. I can look up when it 
 got introduced and I would not be surprised if it's since KDE 2.0. Given that 
 and no translators ever complained, I wouldn't change it ;-)

just for the fun I looked into the history and found the following in 
tabbox.cpp:

+   if (client-isIconified())
+   s += QString(()+client-caption()+);
+   else

now guess the commit :-)


commit 311db796c68e520e5b6d28829d6aaa4bfbcd1536
Author: Matthias Ettrich ettr...@troll.no
Date:   Thu Aug 19 23:26:42 1999 +

Say hello to kwin. WARNING: NOT USABLE YET. See README.

svn path=/trunk/kdebase/kwin/; revision=27871


- Martin


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/109832/#review69422
---


On Oct. 29, 2014, 4:22 p.m., Andre Heinecke wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/109832/
 ---
 
 (Updated Oct. 29, 2014, 4:22 p.m.)
 
 
 Review request for kwin, Plasma and Martin Gräßlin.
 
 
 Bugs: 292566
 http://bugs.kde.org/show_bug.cgi?id=292566
 
 
 Repository: kde-workspace
 
 
 Description
 ---
 
 I'm reopening this review request as I have updated this Window Switcher for 
 Plasma 5.1 and would like to get another review to check if there are any 
 suggestions or issues regarding the port to the new API.
 
 Secondly I would like to ask again to have this Window Switcher Layout 
 included in the KWin repository. I would prefer it if users could obtain this 
 layout from their trusted distributors and did not have to rely on an 
 unverifyable third party download to obtain this plugin. 
 
 As suggested in the original review I've put this up on kde-look and recieved 
 some positive feedback. But I really feel that it is rotting away there and 
 that KDE-Look is not a good place to distribute executable plugins.
 
 IMHO the approach of this Window Switcher is different enough from the others 
 included in KWin to be a useful addition to the fold. Especially as this 
 behavior is already familiar to KDE users from some versions  4.6
 
 It should also be close enough to the other layouts like thumbnails to keep 
 maintenance very similar (I've mostly looked at the changes made to 
 thumbnails to adapt this for Plasma 5)
 
 
 Original description:
 
 This Layout tries to mimic some of the old KDE 4.6 tabbox behavior and 
 layout, it scales the thumbnails shown in the tabbox to avoid scrolling.
 There are also three different states in this layout depending on the size of 
 the scaled thumbnails to provide appropriate information even when there are 
 many opened windows.
 
 States:
 1. Thumbnails are larger then 200px: Show the Title and the Icon of the 
 Window directly below the thumbnail.
 2. Thumbnails are between 200px and 64px: Thumbnails are shown together with 
 the icon but only the title of the currently selected window is shown.
 3. Thumbnails would be smaller then 64px: Only the Icons of the windows are 
 shown and the title of the currently selected window (like big icons)
 If the Thumbnails would be smaller then 32px the tabbox starts to scroll in 
 the Icon Only state.
 
 Size of the thumbnails depends on the screen size and the number of opened 
 windows.
 
 
 Diffs
 -
 
   tabbox/qml/CMakeLists.txt fc55ab9 
   tabbox/qml/clients/scaling/contents/ui/main.qml PRE-CREATION 
   tabbox/qml/clients/scaling/metadata.desktop PRE-CREATION 
 
 Diff: https://git.reviewboard.kde.org/r/109832/diff/
 
 
 Testing
 ---
 
 Tested with plasma 5.3.1 from Kubuntu next / unstable repositories.
 
 
 Thanks,
 
 Andre Heinecke
 


___
Plasma-devel mailing list
Plasma-devel@kde.org

Review Request 120891: Remove unused logout effects in KSMServer

2014-10-29 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120891/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

Remove unused logout effects in KSMServer

KWin handles logout effects, it's not complied at the moment and it
won't ever be again.


Diffs
-

  ksmserver/CMakeLists.txt 083c129 
  ksmserver/curtaineffect.h 32b1f37 
  ksmserver/curtaineffect.cpp 52751e8 
  ksmserver/fadeeffect.h 2bba96d 
  ksmserver/fadeeffect.cpp 3138f65 
  ksmserver/logouteffect.h 59c1aca 
  ksmserver/logouteffect.cpp cafe452 

Diff: https://git.reviewboard.kde.org/r/120891/diff/


Testing
---


Thanks,

David Edmundson

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120891: Remove unused logout effects in KSMServer

2014-10-29 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120891/#review69473
---

Ship it!


+1 that's in the category of world cultural heritage code

- Marco Martin


On Ott. 29, 2014, 5:14 p.m., David Edmundson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120891/
 ---
 
 (Updated Ott. 29, 2014, 5:14 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 Remove unused logout effects in KSMServer
 
 KWin handles logout effects, it's not complied at the moment and it
 won't ever be again.
 
 
 Diffs
 -
 
   ksmserver/CMakeLists.txt 083c129 
   ksmserver/curtaineffect.h 32b1f37 
   ksmserver/curtaineffect.cpp 52751e8 
   ksmserver/fadeeffect.h 2bba96d 
   ksmserver/fadeeffect.cpp 3138f65 
   ksmserver/logouteffect.h 59c1aca 
   ksmserver/logouteffect.cpp cafe452 
 
 Diff: https://git.reviewboard.kde.org/r/120891/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David Edmundson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120892: move notification out of the way of other plasma windows

2014-10-29 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120892/
---

Review request for Plasma.


Repository: plasma-workspace


Description
---

This is an attempt to solve
https://bugs.kde.org/show_bug.cgi?id=338946

It's very crude (therefore no, i don't really like it that much)

alternatively, a second approach i was thinking to: it may be tried to find the 
actual window of the systray popup, it would probably have a better behavior 
(notifications could be moved as well when the popup opens/closes) finding that 
window reliably could be quite messy tough


Diffs
-

  applets/notifications/plugin/notificationshelper.cpp 425f0d6 

Diff: https://git.reviewboard.kde.org/r/120892/diff/


Testing
---


Thanks,

Marco Martin

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120891: Remove unused logout effects in KSMServer

2014-10-29 Thread Martin Gräßlin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120891/#review69474
---

Ship it!


Ship It!

- Martin Gräßlin


On Oct. 29, 2014, 6:14 p.m., David Edmundson wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120891/
 ---
 
 (Updated Oct. 29, 2014, 6:14 p.m.)
 
 
 Review request for Plasma.
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 Remove unused logout effects in KSMServer
 
 KWin handles logout effects, it's not complied at the moment and it
 won't ever be again.
 
 
 Diffs
 -
 
   ksmserver/CMakeLists.txt 083c129 
   ksmserver/curtaineffect.h 32b1f37 
   ksmserver/curtaineffect.cpp 52751e8 
   ksmserver/fadeeffect.h 2bba96d 
   ksmserver/fadeeffect.cpp 3138f65 
   ksmserver/logouteffect.h 59c1aca 
   ksmserver/logouteffect.cpp cafe452 
 
 Diff: https://git.reviewboard.kde.org/r/120891/diff/
 
 
 Testing
 ---
 
 
 Thanks,
 
 David Edmundson
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120806: Port fifteenPuzzle applet to qml and plasma 5.

2014-10-29 Thread David Edmundson

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120806/#review69475
---

Ship it!


Ship It!

- David Edmundson


On Oct. 28, 2014, 5:48 p.m., Jeremy Whiting wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120806/
 ---
 
 (Updated Oct. 28, 2014, 5:48 p.m.)
 
 
 Review request for Plasma and David Edmundson.
 
 
 Repository: kdeplasma-addons
 
 
 Description
 ---
 
 Config options from previous c++ version kept though names are a bit diferent.
 Images not yet supported.
 Puzzle starts at 0 in the top left corner, maybe should start with 1 in the 
 corner though.
 
 This is my first port to plasma qml, so I may have used some of the non 
 suggested/recommended components, if so let me know and I'll fix it.
 
 
 Diffs
 -
 
   applets/CMakeLists.txt 63e6e25628d18ee474231acd2a21711841dee592 
   applets/fifteenPuzzle/CMakeLists.txt 
 04d5e55fd246684855d49484f1233dac054a0124 
   applets/fifteenPuzzle/Messages.sh PRE-CREATION 
   applets/fifteenPuzzle/icons/CMakeLists.txt 
 106884f432c1d1e0b0584959af854c79ede4ea6d 
   applets/fifteenPuzzle/icons/hisc-app-fifteenpuzzle.svgz  
   applets/fifteenPuzzle/images/blanksquare.svg  
   applets/fifteenPuzzle/package/contents/config/config.qml PRE-CREATION 
   applets/fifteenPuzzle/package/contents/config/main.xml PRE-CREATION 
   applets/fifteenPuzzle/package/contents/ui/ColorPicker.qml PRE-CREATION 
   applets/fifteenPuzzle/package/contents/ui/FifteenPuzzle.qml PRE-CREATION 
   applets/fifteenPuzzle/package/contents/ui/Piece.qml PRE-CREATION 
   applets/fifteenPuzzle/package/contents/ui/configAppearance.qml PRE-CREATION 
   applets/fifteenPuzzle/package/contents/ui/main.qml PRE-CREATION 
   applets/fifteenPuzzle/plasma-applet-fifteenPuzzle.desktop 
 513cc0084df7247a520807620361b0426623727e 
   applets/fifteenPuzzle/src/Messages.sh 
 bab24ae73049f37d9693cf062eaaa98ca1e6bab0 
   applets/fifteenPuzzle/src/fifteen.h 
 2a27f5b109988003de45fb64c457484ebdfdbc8b 
   applets/fifteenPuzzle/src/fifteen.cpp 
 ebdcf2c0756a17ea174c0fc5fd106e157b223063 
   applets/fifteenPuzzle/src/fifteenPuzzle.h 
 cf7885380f0e152d51cf2dc7557444e9b425b596 
   applets/fifteenPuzzle/src/fifteenPuzzle.cpp 
 8a1528988f3e693d20179db4a209309b0aad87fd 
   applets/fifteenPuzzle/src/fifteenPuzzleConfig.ui 
 ff82f331db4cee2d66f526954be63f0f5d81d250 
   applets/fifteenPuzzle/src/piece.h d0e58d0f9d38d4a1ef2110b974b3f4f6938293e1 
   applets/fifteenPuzzle/src/piece.cpp 
 2efb72ecf69d9beaa53367bc2f3c9cee88238f28 
   applets/systemloadviewer/package/contents/ui/ColorPicker.qml 
 92062db546dcff67f930d4888180f4e753798c27 
 
 Diff: https://git.reviewboard.kde.org/r/120806/diff/
 
 
 Testing
 ---
 
 I've tested it with plasmoidviewer -a org.kde.plasma.fifteenpuzzle. 
 
 The icon is not working in the widget adder, not sure where to install that 
 to for it to work.
 Images not supported yet, though they are in the config, maybe should remove 
 from config until they are supported?
 
 
 Thanks,
 
 Jeremy Whiting
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120806: Port fifteenPuzzle applet to qml and plasma 5.

2014-10-29 Thread Jeremy Whiting

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120806/
---

(Updated Oct. 29, 2014, 6:23 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and David Edmundson.


Repository: kdeplasma-addons


Description
---

Config options from previous c++ version kept though names are a bit diferent.
Images not yet supported.
Puzzle starts at 0 in the top left corner, maybe should start with 1 in the 
corner though.

This is my first port to plasma qml, so I may have used some of the non 
suggested/recommended components, if so let me know and I'll fix it.


Diffs
-

  applets/CMakeLists.txt 63e6e25628d18ee474231acd2a21711841dee592 
  applets/fifteenPuzzle/CMakeLists.txt 04d5e55fd246684855d49484f1233dac054a0124 
  applets/fifteenPuzzle/Messages.sh PRE-CREATION 
  applets/fifteenPuzzle/icons/CMakeLists.txt 
106884f432c1d1e0b0584959af854c79ede4ea6d 
  applets/fifteenPuzzle/icons/hisc-app-fifteenpuzzle.svgz  
  applets/fifteenPuzzle/images/blanksquare.svg  
  applets/fifteenPuzzle/package/contents/config/config.qml PRE-CREATION 
  applets/fifteenPuzzle/package/contents/config/main.xml PRE-CREATION 
  applets/fifteenPuzzle/package/contents/ui/ColorPicker.qml PRE-CREATION 
  applets/fifteenPuzzle/package/contents/ui/FifteenPuzzle.qml PRE-CREATION 
  applets/fifteenPuzzle/package/contents/ui/Piece.qml PRE-CREATION 
  applets/fifteenPuzzle/package/contents/ui/configAppearance.qml PRE-CREATION 
  applets/fifteenPuzzle/package/contents/ui/main.qml PRE-CREATION 
  applets/fifteenPuzzle/plasma-applet-fifteenPuzzle.desktop 
513cc0084df7247a520807620361b0426623727e 
  applets/fifteenPuzzle/src/Messages.sh 
bab24ae73049f37d9693cf062eaaa98ca1e6bab0 
  applets/fifteenPuzzle/src/fifteen.h 2a27f5b109988003de45fb64c457484ebdfdbc8b 
  applets/fifteenPuzzle/src/fifteen.cpp 
ebdcf2c0756a17ea174c0fc5fd106e157b223063 
  applets/fifteenPuzzle/src/fifteenPuzzle.h 
cf7885380f0e152d51cf2dc7557444e9b425b596 
  applets/fifteenPuzzle/src/fifteenPuzzle.cpp 
8a1528988f3e693d20179db4a209309b0aad87fd 
  applets/fifteenPuzzle/src/fifteenPuzzleConfig.ui 
ff82f331db4cee2d66f526954be63f0f5d81d250 
  applets/fifteenPuzzle/src/piece.h d0e58d0f9d38d4a1ef2110b974b3f4f6938293e1 
  applets/fifteenPuzzle/src/piece.cpp 2efb72ecf69d9beaa53367bc2f3c9cee88238f28 
  applets/systemloadviewer/package/contents/ui/ColorPicker.qml 
92062db546dcff67f930d4888180f4e753798c27 

Diff: https://git.reviewboard.kde.org/r/120806/diff/


Testing
---

I've tested it with plasmoidviewer -a org.kde.plasma.fifteenpuzzle. 

The icon is not working in the widget adder, not sure where to install that to 
for it to work.
Images not supported yet, though they are in the config, maybe should remove 
from config until they are supported?


Thanks,

Jeremy Whiting

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 120894: Forward wheel events inside panelview

2014-10-29 Thread Kai Uwe Broulik

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120894/
---

Review request for Plasma.


Bugs: 340412
https://bugs.kde.org/show_bug.cgi?id=340412


Repository: plasma-workspace


Description
---

This makes it also forward wheel events in the same fashion as it does with 
clicks. Restores Fitt's law for taskmanager wheeling. A similar patch for 
plasma-frameworks dialog is needed as well.


Diffs
-

  shell/panelview.cpp 2bece00 

Diff: https://git.reviewboard.kde.org/r/120894/diff/


Testing
---

Throw your mouse to the edge where your taskmanager is and wheel. Works as 
expected.


Thanks,

Kai Uwe Broulik

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120894: Forward wheel events inside panelview

2014-10-29 Thread Marco Martin

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120894/#review69482
---

Ship it!


good apart a small issue.
Also, this should be done in Dialog as well, keeping the event forward identical


shell/panelview.cpp
https://git.reviewboard.kde.org/r/120894/#comment48643

would be cleaner to put it in its own case, instead of grouped together 
with the mouse events, since is completely separed anyways


- Marco Martin


On Ott. 29, 2014, 7:33 p.m., Kai Uwe Broulik wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120894/
 ---
 
 (Updated Ott. 29, 2014, 7:33 p.m.)
 
 
 Review request for Plasma.
 
 
 Bugs: 340412
 https://bugs.kde.org/show_bug.cgi?id=340412
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 This makes it also forward wheel events in the same fashion as it does with 
 clicks. Restores Fitt's law for taskmanager wheeling. A similar patch for 
 plasma-frameworks dialog is needed as well.
 
 
 Diffs
 -
 
   shell/panelview.cpp 2bece00 
 
 Diff: https://git.reviewboard.kde.org/r/120894/diff/
 
 
 Testing
 ---
 
 Throw your mouse to the edge where your taskmanager is and wheel. Works as 
 expected.
 
 
 Thanks,
 
 Kai Uwe Broulik
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Hrvoje Senjan


 On Oct. 29, 2014, 1:41 p.m., Thomas Lübking wrote:
  The version check is actually on the bugzilla version (KDE_MAKE_VERSION is 
  just a bitshifting macro) - it's the important part in the original patch 
  ;-)
 
 Thomas Lübking wrote:
 To prevent future confusion, one might want to add
 
 #define KDE_MAKE_BUGZILLA_VERSION KDE_MAKE_VERSION
 
 or sth. like this.

note to self: never blindly c/p pathes ;-)
still, i wonder, how will those versions get evaluated if building with kdelibs 
 4.5.x ?
i guess i should read the patch and discussion more closely.


- Hrvoje


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/#review69429
---


On Oct. 29, 2014, 12:02 a.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120876/
 ---
 
 (Updated Oct. 29, 2014, 12:02 a.m.)
 
 
 Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.
 
 
 Bugs: 337742
 https://bugs.kde.org/show_bug.cgi?id=337742
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 discussion was in https://git.reviewboard.kde.org/r/120431/
 removed the version checks, as we know we have kdelibs = 4.5 ;-)
 
 
 Diffs
 -
 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 
   drkonqi/bugzillaintegration/bugzillalib.h 570169b 
   drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
 
 Diff: https://git.reviewboard.kde.org/r/120876/diff/
 
 
 Testing
 ---
 
 builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
 before.
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Thomas Lübking


 On Okt. 29, 2014, 12:41 nachm., Thomas Lübking wrote:
  The version check is actually on the bugzilla version (KDE_MAKE_VERSION is 
  just a bitshifting macro) - it's the important part in the original patch 
  ;-)
 
 Thomas Lübking wrote:
 To prevent future confusion, one might want to add
 
 #define KDE_MAKE_BUGZILLA_VERSION KDE_MAKE_VERSION
 
 or sth. like this.
 
 Hrvoje Senjan wrote:
 note to self: never blindly c/p pathes ;-)
 still, i wonder, how will those versions get evaluated if building with 
 kdelibs  4.5.x ?
 i guess i should read the patch and discussion more closely.

the numbers refer to bugzilla 4.5.x - KDE_MAKE_VERSION(a,b,c) is just a16 
| b8 | c (no, i don't know why it wastes 8 bits ;-)
You'd typically test against KDE_VERSION or directly use KDE_IS_VERSION to 
actually compare the runtime KDE library.

This patch however tests against numbers it queries from the bugzilla server.


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/#review69429
---


On Okt. 28, 2014, 11:02 nachm., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120876/
 ---
 
 (Updated Okt. 28, 2014, 11:02 nachm.)
 
 
 Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.
 
 
 Bugs: 337742
 https://bugs.kde.org/show_bug.cgi?id=337742
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 discussion was in https://git.reviewboard.kde.org/r/120431/
 removed the version checks, as we know we have kdelibs = 4.5 ;-)
 
 
 Diffs
 -
 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 
   drkonqi/bugzillaintegration/bugzillalib.h 570169b 
   drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
 
 Diff: https://git.reviewboard.kde.org/r/120876/diff/
 
 
 Testing
 ---
 
 builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
 before.
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120894: Forward wheel events inside panelview

2014-10-29 Thread Kai Uwe Broulik


 On Okt. 29, 2014, 7:55 nachm., Marco Martin wrote:
  shell/panelview.cpp, line 688
  https://git.reviewboard.kde.org/r/120894/diff/1/?file=323565#file323565line688
 
  would be cleaner to put it in its own case, instead of grouped together 
  with the mouse events, since is completely separed anyways

QWheelEvent doesn't have a windowPos() which is why I put it in there. Or 
should I make a separate case and have a cast to QMouseEvent and QWheelEvent 
there?


- Kai Uwe


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120894/#review69482
---


On Okt. 29, 2014, 7:33 nachm., Kai Uwe Broulik wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120894/
 ---
 
 (Updated Okt. 29, 2014, 7:33 nachm.)
 
 
 Review request for Plasma.
 
 
 Bugs: 340412
 https://bugs.kde.org/show_bug.cgi?id=340412
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 This makes it also forward wheel events in the same fashion as it does with 
 clicks. Restores Fitt's law for taskmanager wheeling. A similar patch for 
 plasma-frameworks dialog is needed as well.
 
 
 Diffs
 -
 
   shell/panelview.cpp 2bece00 
 
 Diff: https://git.reviewboard.kde.org/r/120894/diff/
 
 
 Testing
 ---
 
 Throw your mouse to the edge where your taskmanager is and wheel. Works as 
 expected.
 
 
 Thanks,
 
 Kai Uwe Broulik
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120894: Forward wheel events inside panelview

2014-10-29 Thread Marco Martin


 On Ott. 29, 2014, 7:55 p.m., Marco Martin wrote:
  shell/panelview.cpp, line 688
  https://git.reviewboard.kde.org/r/120894/diff/1/?file=323565#file323565line688
 
  would be cleaner to put it in its own case, instead of grouped together 
  with the mouse events, since is completely separed anyways
 
 Kai Uwe Broulik wrote:
 QWheelEvent doesn't have a windowPos() which is why I put it in there. Or 
 should I make a separate case and have a cast to QMouseEvent and QWheelEvent 
 there?

It has globalPos() that can be used.

the fact that you are casting the event both to wheelevent and mouseevent seems 
to be quite by chance that's not crashing, since a wheelevent is not a 
mouseevent subclass (but just inputevent common ancestor)


- Marco


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120894/#review69482
---


On Ott. 29, 2014, 7:33 p.m., Kai Uwe Broulik wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120894/
 ---
 
 (Updated Ott. 29, 2014, 7:33 p.m.)
 
 
 Review request for Plasma.
 
 
 Bugs: 340412
 https://bugs.kde.org/show_bug.cgi?id=340412
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 This makes it also forward wheel events in the same fashion as it does with 
 clicks. Restores Fitt's law for taskmanager wheeling. A similar patch for 
 plasma-frameworks dialog is needed as well.
 
 
 Diffs
 -
 
   shell/panelview.cpp 2bece00 
 
 Diff: https://git.reviewboard.kde.org/r/120894/diff/
 
 
 Testing
 ---
 
 Throw your mouse to the edge where your taskmanager is and wheel. Works as 
 expected.
 
 
 Thanks,
 
 Kai Uwe Broulik
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Hrvoje Senjan


 On Oct. 29, 2014, 1:41 p.m., Thomas Lübking wrote:
  The version check is actually on the bugzilla version (KDE_MAKE_VERSION is 
  just a bitshifting macro) - it's the important part in the original patch 
  ;-)
 
 Thomas Lübking wrote:
 To prevent future confusion, one might want to add
 
 #define KDE_MAKE_BUGZILLA_VERSION KDE_MAKE_VERSION
 
 or sth. like this.
 
 Hrvoje Senjan wrote:
 note to self: never blindly c/p pathes ;-)
 still, i wonder, how will those versions get evaluated if building with 
 kdelibs  4.5.x ?
 i guess i should read the patch and discussion more closely.
 
 Thomas Lübking wrote:
 the numbers refer to bugzilla 4.5.x - KDE_MAKE_VERSION(a,b,c) is just 
 a16 | b8 | c (no, i don't know why it wastes 8 bits ;-)
 You'd typically test against KDE_VERSION or directly use KDE_IS_VERSION 
 to actually compare the runtime KDE library.
 
 This patch however tests against numbers it queries from the bugzilla 
 server.

in order not to put back in dependancy to kdelibs4support, going to try with 
QT_VERSION_CHECK. hopefully to have same effect


- Hrvoje


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/#review69429
---


On Oct. 29, 2014, 12:02 a.m., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120876/
 ---
 
 (Updated Oct. 29, 2014, 12:02 a.m.)
 
 
 Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.
 
 
 Bugs: 337742
 https://bugs.kde.org/show_bug.cgi?id=337742
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 discussion was in https://git.reviewboard.kde.org/r/120431/
 removed the version checks, as we know we have kdelibs = 4.5 ;-)
 
 
 Diffs
 -
 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 
   drkonqi/bugzillaintegration/bugzillalib.h 570169b 
   drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
 
 Diff: https://git.reviewboard.kde.org/r/120876/diff/
 
 
 Testing
 ---
 
 builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
 before.
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Hrvoje Senjan

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/
---

(Updated Oct. 29, 2014, 9:25 p.m.)


Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.


Changes
---

restore version check


Bugs: 337742
https://bugs.kde.org/show_bug.cgi?id=337742


Repository: plasma-workspace


Description
---

discussion was in https://git.reviewboard.kde.org/r/120431/
removed the version checks, as we know we have kdelibs = 4.5 ;-)


Diffs (updated)
-

  drkonqi/bugzillaintegration/bugzillalib.h 570169b 
  drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
  drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
  drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 

Diff: https://git.reviewboard.kde.org/r/120876/diff/


Testing
---

builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
before.


Thanks,

Hrvoje Senjan

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Thomas Lübking


 On Okt. 29, 2014, 12:41 nachm., Thomas Lübking wrote:
  The version check is actually on the bugzilla version (KDE_MAKE_VERSION is 
  just a bitshifting macro) - it's the important part in the original patch 
  ;-)
 
 Thomas Lübking wrote:
 To prevent future confusion, one might want to add
 
 #define KDE_MAKE_BUGZILLA_VERSION KDE_MAKE_VERSION
 
 or sth. like this.
 
 Hrvoje Senjan wrote:
 note to self: never blindly c/p pathes ;-)
 still, i wonder, how will those versions get evaluated if building with 
 kdelibs  4.5.x ?
 i guess i should read the patch and discussion more closely.
 
 Thomas Lübking wrote:
 the numbers refer to bugzilla 4.5.x - KDE_MAKE_VERSION(a,b,c) is just 
 a16 | b8 | c (no, i don't know why it wastes 8 bits ;-)
 You'd typically test against KDE_VERSION or directly use KDE_IS_VERSION 
 to actually compare the runtime KDE library.
 
 This patch however tests against numbers it queries from the bugzilla 
 server.
 
 Hrvoje Senjan wrote:
 in order not to put back in dependancy to kdelibs4support, going to try 
 with QT_VERSION_CHECK. hopefully to have same effect

Yes, has.
However, now it reads even more misleading.

Maybe put

#define MAKE_BUGZILLA_VERSION(a,b,c) (((a)  16) | ((b)  8) | (c))

below the includes and use that to prevent future confusion?


- Thomas


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/#review69429
---


On Okt. 29, 2014, 8:25 nachm., Hrvoje Senjan wrote:
 
 ---
 This is an automatically generated e-mail. To reply, visit:
 https://git.reviewboard.kde.org/r/120876/
 ---
 
 (Updated Okt. 29, 2014, 8:25 nachm.)
 
 
 Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.
 
 
 Bugs: 337742
 https://bugs.kde.org/show_bug.cgi?id=337742
 
 
 Repository: plasma-workspace
 
 
 Description
 ---
 
 discussion was in https://git.reviewboard.kde.org/r/120431/
 removed the version checks, as we know we have kdelibs = 4.5 ;-)
 
 
 Diffs
 -
 
   drkonqi/bugzillaintegration/bugzillalib.h 570169b 
   drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
   drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 
 
 Diff: https://git.reviewboard.kde.org/r/120876/diff/
 
 
 Testing
 ---
 
 builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
 before.
 
 
 Thanks,
 
 Hrvoje Senjan
 


___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120876: Forward-port Fix and future-proof Dr Konqi security methods on Bugzilla from kde-runtime

2014-10-29 Thread Hrvoje Senjan

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120876/
---

(Updated Oct. 29, 2014, 9:41 p.m.)


Review request for Plasma, Ben Cooksley, Ian Wadham, and Thomas Lübking.


Changes
---

use an internal define instead of existing/external ones to prevent future 
confusion


Bugs: 337742
https://bugs.kde.org/show_bug.cgi?id=337742


Repository: plasma-workspace


Description
---

discussion was in https://git.reviewboard.kde.org/r/120431/
removed the version checks, as we know we have kdelibs = 4.5 ;-)


Diffs (updated)
-

  drkonqi/bugzillaintegration/bugzillalib.h 570169b 
  drkonqi/bugzillaintegration/bugzillalib.cpp 8fd8399 
  drkonqi/bugzillaintegration/reportassistantpages_bugzilla.h 50cf05f 
  drkonqi/bugzillaintegration/reportassistantpages_bugzilla.cpp 5a6096f 

Diff: https://git.reviewboard.kde.org/r/120876/diff/


Testing
---

builds, succesfully reported bug via patched DrKonqi, wasn't able to do so 
before.


Thanks,

Hrvoje Senjan

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120894: Forward wheel events inside panelview

2014-10-29 Thread Kai Uwe Broulik

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120894/
---

(Updated Okt. 29, 2014, 8:56 nachm.)


Review request for Plasma.


Changes
---

Use separate branch

There seems to be something wrong with the margins (unrelated to this), 
however. In a bottom panel I can wheel at the bottom of the screen without a 
problem, in a left and top panel it does not work without this patch.


Bugs: 340412
https://bugs.kde.org/show_bug.cgi?id=340412


Repository: plasma-workspace


Description
---

This makes it also forward wheel events in the same fashion as it does with 
clicks. Restores Fitt's law for taskmanager wheeling. A similar patch for 
plasma-frameworks dialog is needed as well.


Diffs (updated)
-

  shell/panelview.cpp 2bece00 

Diff: https://git.reviewboard.kde.org/r/120894/diff/


Testing
---

Throw your mouse to the edge where your taskmanager is and wheel. Works as 
expected.


Thanks,

Kai Uwe Broulik

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 120877: Convert switch statements to if/else due to MSVC limitation

2014-10-29 Thread Andrius da Costa Ribas

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/120877/
---

(Updated Out. 30, 2014, 1:35 a.m.)


Review request for kdewin and Plasma.


Changes
---

Fix coding style.


Repository: plasma-framework


Description
---

MSVC is unable to recognise e.g. FrameSvg::TopBorder | FrameSvg::LeftBorder as 
a compile-time const expression (error C2051: case expression not constant)


Diffs (updated)
-

  src/plasma/private/framesvg_helpers.h 5b96cd5 

Diff: https://git.reviewboard.kde.org/r/120877/diff/


Testing
---

Builds with MSVC 2013 64bit.

framesvg related tests pass, other unrelated tests still fail.

Test project V:/build/frameworks/plasma/work/msvc2013-RelWithDebInfo-master
  Start  1: plasma-dialogqmltest
 1/10 Test  #1: plasma-dialogqmltest ...***Failed1.15 sec
  Start  2: plasma-fallbackpackagetest
 2/10 Test  #2: plasma-fallbackpackagetest .   Passed0.16 sec
  Start  3: plasma-packagestructuretest
 3/10 Test  #3: plasma-packagestructuretest ***Failed0.19 sec
  Start  4: plasma-packageurlinterceptortest
 4/10 Test  #4: plasma-packageurlinterceptortest ...   Passed0.12 sec
  Start  5: plasma-pluginloadertest
 5/10 Test  #5: plasma-pluginloadertest    Passed1.44 sec
  Start  6: plasma-framesvgtest
 6/10 Test  #6: plasma-framesvgtest    Passed0.64 sec
  Start  7: coronatest
 7/10 Test  #7: coronatest .***Failed0.16 sec
  Start  8: plasma-storagetest
 8/10 Test  #8: plasma-storagetest .***Failed0.12 sec
  Start  9: plasma-sortfiltermodeltest
 9/10 Test  #9: plasma-sortfiltermodeltest .   Passed0.14 sec
  Start 10: i18ndcheck
Could not find executable SH-NOTFOUND
Looked in the following places:
SH-NOTFOUND
SH-NOTFOUND.exe
Release/SH-NOTFOUND
Release/SH-NOTFOUND.exe
Debug/SH-NOTFOUND
Debug/SH-NOTFOUND.exe
MinSizeRel/SH-NOTFOUND
MinSizeRel/SH-NOTFOUND.exe
RelWithDebInfo/SH-NOTFOUND
RelWithDebInfo/SH-NOTFOUND.exe
Deployment/SH-NOTFOUND
Deployment/SH-NOTFOUND.exe
Development/SH-NOTFOUND
Development/SH-NOTFOUND.exe
Unable to find executable: SH-NOTFOUND
10/10 Test #10: i18ndcheck .***Not Run   0.00 sec

50% tests passed, 5 tests failed out of 10


Thanks,

Andrius da Costa Ribas

___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel