Re: Review Request 125793: [Task Manager] Add proper close button to tooltip

2015-10-26 Thread Eike Hein


> On Oct. 25, 2015, 11:08 p.m., Uri Herrera wrote:
> > I agree with Thomas that having the button inside the thumbnail is 
> > confusing.
> > 
> > ![](http://i.imgur.com/JlZveha.png)
> > 
> > On this image I moved the button outside the thumbnail, used the actual 
> > window-close icon in the icon theme and removed the round button background.
> > 
> > ![](http://i.imgur.com/1gfju2e.png)
> > 
> > On this the button is inside and there's no button background.
> 
> Uri Herrera wrote:
> I also made the button bigger, currently the button looks really tiny for 
> me.

The button is mostly on the thumbnail because there could be multiple windows 
in the tooltip.


- Eike


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


On Oct. 25, 2015, 9:42 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125793/
> ---
> 
> (Updated Oct. 25, 2015, 9:42 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 350705
> https://bugs.kde.org/show_bug.cgi?id=350705
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> I just found this patch I made a while ago. This turns the handmade close 
> button into a regular ToolButton (which is only shown when hovering) with a 
> proper size. Also, the button is always in the top right corner of the 
> *thumbnail* rather than the dialog
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/ToolTipDelegate.qml f7084c7 
> 
> Diff: https://git.reviewboard.kde.org/r/125793/diff/
> 
> 
> Testing
> ---
> 
> Works.
> 
> 
> File Attachments
> 
> 
> New close button
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/25/0fea55c5-5a9b-4321-bec5-34ec3bacf239__closebuttonproper.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 125805: Improve lockwindow test

2015-10-26 Thread Martin Gräßlin

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


please verify that the background is colored at the expected places.

- Martin Gräßlin


On Oct. 26, 2015, 10:18 a.m., Bhushan Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125805/
> ---
> 
> (Updated Oct. 26, 2015, 10:18 a.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Earlier we were creating dummy window but it was just black/transperant, so 
> given that isBlack check would succeed even if lockwindow is not shown in 
> some cases.
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
> 
> Diff: https://git.reviewboard.kde.org/r/125805/diff/
> 
> 
> Testing
> ---
> 
> tests pass
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Eike Hein


> On Oct. 25, 2015, 10:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.

Thomas, all this does is not grey out the text in Task Manager button labels 
when a window is minimized. See also the long discussion in the referenced bug 
ticket.


- Eike


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


On Oct. 23, 2015, 11:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 11:16 a.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml 
> f655801ab1f7b1a9a915f35149c858f0ede22a29 
> 
> Diff: https://git.reviewboard.kde.org/r/124675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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


Minutes Monday Plasma Hangout

2015-10-26 Thread Martin Graesslin
Present: Martin G, bhushan, david, jonathan, Kai Uwe, Marco, sebas

today protocol from me as sebas had kdenlive running causing weirdness. If 
something is missing, please add

Date: 2015-10-26

Bhushan:
- screen locker for Wayland https://git.reviewboard.kde.org/r/125802/

David:
- missing icons in systray (QML weirdness)
- 30 sec freeze during startup in Kubuntu due to bluetooth, Jan fixed it

Jonathan:
- work on breeze_gtk theme

Kai:
- webp mimetype detection
- lockscreen with new sessions model
- not yet merged, will go together with all user switching related stuff (user 
switcher model, user switcher plasmoid)
- Task Manager close button: https://git.reviewboard.kde.org/r/125793/

Marco:
- away most of week
- work on new theme (it's in a branch and will be merged soonish)
- work on documentation for designers

Sebas:
- away most of week
- screen management for Wayland

Martin G:
- work on Wayland geometry handling: unrestricted move of Wayland clients work
- fixed 4 QtQuick related crashers, weirdness going on in Systemsettings + 
QtQuick

signature.asc
Description: This is a digitally signed message part.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[Plasma Workspace Wallpapers] [Bug 354379] Wallpaper fails to rotate 90 on dual screen with external scr. on left. Stays horiz. and spills over onto right scr.

2015-10-26 Thread PhillB via KDE Bugzilla
https://bugs.kde.org/show_bug.cgi?id=354379

--- Comment #2 from PhillB  ---
BTW. It used to work fine on Fedora 21.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 125805: Improve lockwindow test

2015-10-26 Thread Bhushan Shah

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

(Updated Oct. 26, 2015, 9:57 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Martin Gräßlin.


Changes
---

Submitted with commit 9bf61a335b06b94aae642518bbe1d7f671880061 by Bhushan Shah 
to branch master.


Repository: plasma-workspace


Description
---

Earlier we were creating dummy window but it was just black/transperant, so 
given that isBlack check would succeed even if lockwindow is not shown in some 
cases.


Diffs
-

  ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 

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


Testing
---

tests pass


Thanks,

Bhushan Shah

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


Re: RFC: Put notifications in sidepanel like widget explorer

2015-10-26 Thread Sebastian Kügler
On Tuesday, October 20, 2015 15:54:01 Martin Klapetek wrote:
> today I was thinking a bit about our notification system. I believe
> that the constraints the tiny popup give us are just too limiting.
> Currently it fits about two notifications and for everything else
> one has to scroll, plus the space is really cramped for anything
> else besides the notifications themselves.
> 
> So it occurred to me that we could maybe reuse the same side
> panel we use for widget explorer and activities, only coming in
> from the right side.
> 
> This would give us much more space and with that many more
> possibilities, like proper grouping of notifications from different
> apps and possibly also putting /all/ notifications there again 
> (I'm still against it but with that extra space, we could afford it).

I can't remember a single case in the last months of usage (except for one, 
see below) that made me want that, notifications are already cleaned up, and I 
simply don't get that many persistent ones.

On the other hand, I find a side-panel much more intrusive, and much less 
quick-peak-dismiss, so it makes the ui and workflow feel heavier.

The case I did have multiple persistent notifications is only after multiple 
transfers, where the "transferring that file is finished" notifications 
persisted.

Perhaps we can instead look into the notifications that stick around and do a 
deeper analysis of where they come from, what they have in common, and how we 
can present that better. More spacing and grouping sounds like a very 
technocratic solution to me. I'd rather have the notification system be 
smarter, than presented in a different way, since that would make the 
improvements transferable to other formfactors as well.

> Basically the idea is to reinvent the notifications in Plasma a bit
> and create a kind of notification center.
-- 
sebas

Sebastian Kügler|http://vizZzion.org| http://kde.org

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


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread Sebastian Kügler

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



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


Why are you removing this? It seems unrelated to the rest of this patch.


- Sebastian Kügler


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


Re: Review Request 125805: Improve lockwindow test

2015-10-26 Thread Bhushan Shah

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

(Updated Oct. 26, 2015, 3:13 p.m.)


Review request for Plasma and Martin Gräßlin.


Changes
---

now verifies if 0,0,100,100 is red


Repository: plasma-workspace


Description
---

Earlier we were creating dummy window but it was just black/transperant, so 
given that isBlack check would succeed even if lockwindow is not shown in some 
cases.


Diffs (updated)
-

  ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 

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


Testing
---

tests pass


Thanks,

Bhushan Shah

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


Re: Review Request 125805: Improve lockwindow test

2015-10-26 Thread Martin Gräßlin

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

Ship it!


Ship It!

- Martin Gräßlin


On Oct. 26, 2015, 10:43 a.m., Bhushan Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125805/
> ---
> 
> (Updated Oct. 26, 2015, 10:43 a.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Earlier we were creating dummy window but it was just black/transperant, so 
> given that isBlack check would succeed even if lockwindow is not shown in 
> some cases.
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
> 
> Diff: https://git.reviewboard.kde.org/r/125805/diff/
> 
> 
> Testing
> ---
> 
> tests pass
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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


Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Bhushan Shah

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

(Updated Oct. 26, 2015, 1:02 p.m.)


Review request for Plasma and Martin Gräßlin.


Changes
---

Fix issues

- Add previous copyrights
- Don't change position of initialize()

I hope this revision is readable, done with different options.


Repository: plasma-workspace


Description
---

Advantage of this split is some parts such as showLockWindow and hideLockWindow 
needs platform specific code. So instead of doing complex if/else branching we 
can have seperate X11Locker and WaylandLocker.


Diffs (updated)
-

  ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
  ksmserver/screenlocker/CMakeLists.txt 24a89f6 
  ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
  ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
  ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
  ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
  ksmserver/screenlocker/ksldapp.h b281ab6 
  ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
  ksmserver/screenlocker/lockwindow.h c4013be 
  ksmserver/screenlocker/lockwindow.cpp e46733e 
  ksmserver/screenlocker/x11locker.h PRE-CREATION 
  ksmserver/screenlocker/x11locker.cpp PRE-CREATION 

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


Testing
---

builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
also works.


Thanks,

Bhushan Shah

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


Re: Review Request 125771: Use ecm to allow configuring the install location

2015-10-26 Thread Heiko Becker

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

(Updated Oct. 26, 2015, 8:56 a.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 06fdf396b15c62e2d4a7e84272d57a4c65a75b00 by Heiko Becker 
to branch master.


Repository: breeze-gtk


Description
---

This is helpful on a multiarch layout where the prefix is /usr/${host}
but arch-independent files should still be installed to /usr/share.


I do not know if it's acceptable for this project to depend on 
extra-cmake-modules. If not an alternative approach could be introducing a 
variable like SHARE_INSTALL_PREFIX.


Diffs
-

  Breeze-dark-gtk/CMakeLists.txt c1a4c46 
  Breeze-gtk/CMakeLists.txt cbd03aa 
  CMakeLists.txt b977da8 

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


Testing
---

Installed with -DCMAKE_INSTALL_PREFIX:PATH=/usr/x86_64-pc-linux-gnu and 
-DCMAKE_INSTALL_FULL_DATAROOTDIR:PATH=/usr/share/


Thanks,

Heiko Becker

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


Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Martin Gräßlin

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


+1, looks good to me. Let's give it some time for other devs to also have a 
look at it.

- Martin Gräßlin


On Oct. 26, 2015, 9:11 a.m., Bhushan Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125802/
> ---
> 
> (Updated Oct. 26, 2015, 9:11 a.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Advantage of this split is some parts such as showLockWindow and 
> hideLockWindow needs platform specific code. So instead of doing complex 
> if/else branching we can have seperate X11Locker and WaylandLocker.
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/CMakeLists.txt 24a89f6 
>   ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
>   ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
>   ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
>   ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
>   ksmserver/screenlocker/ksldapp.h b281ab6 
>   ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
>   ksmserver/screenlocker/lockwindow.h c4013be 
>   ksmserver/screenlocker/lockwindow.cpp e46733e 
> 
> Diff: https://git.reviewboard.kde.org/r/125802/diff/
> 
> 
> Testing
> ---
> 
> builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
> also works.
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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


Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Martin Gräßlin

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


Difficult to read on reviewboard. Overall it looks good to me, but will need 
another look at it.


ksmserver/screenlocker/lockwindow.h (line 5)


I would keep the previous copyrights. In my opinion they all still have 
copyright on it given that it's based on their work.



ksmserver/screenlocker/lockwindow.h (lines 58 - 59)


why did you switch the position of initialize()?


- Martin Gräßlin


On Oct. 26, 2015, 8:17 a.m., Bhushan Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125802/
> ---
> 
> (Updated Oct. 26, 2015, 8:17 a.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Advantage of this split is some parts such as showLockWindow and 
> hideLockWindow needs platform specific code. So instead of doing complex 
> if/else branching we can have seperate X11Locker and WaylandLocker.
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
>   ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
>   ksmserver/screenlocker/CMakeLists.txt 24a89f6 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
>   ksmserver/screenlocker/ksldapp.h b281ab6 
>   ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
>   ksmserver/screenlocker/lockwindow.h c4013be 
>   ksmserver/screenlocker/lockwindow.h c4013be 
>   ksmserver/screenlocker/lockwindow.cpp e46733e 
> 
> Diff: https://git.reviewboard.kde.org/r/125802/diff/
> 
> 
> Testing
> ---
> 
> builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
> also works.
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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


[Plasma Workspace Wallpapers] [Bug 354379] New: Wallpaper fails to rotate 90 on dual screen with external scr. on left. Stays horiz. and spills over onto right scr.

2015-10-26 Thread PhillB via KDE Bugzilla
https://bugs.kde.org/show_bug.cgi?id=354379

Bug ID: 354379
   Summary: Wallpaper fails to rotate 90 on dual screen with
external scr. on left. Stays horiz. and spills over
onto right scr.
   Product: Plasma Workspace Wallpapers
   Version: 5.4.1
  Platform: Fedora RPMs
OS: Linux
Status: UNCONFIRMED
  Severity: normal
  Priority: NOR
 Component: general
  Assignee: plasma-devel@kde.org
  Reporter: phi...@webwombat.com.au

Laptop has a Benq external monitor plugged in on the left.

The Benq is rotate 90 so it stands tall (portrait).

The wallpaper stays landscape most times. Sometimes it's just black (no
wallpaper). Occassionally it comes good if you logout and then login. Always
portrait on startup.

Programs work fine. I can drag windows over to the left screen and they behave
as expected.

Reproducible: Always

Steps to Reproduce:
1. Plug a monitor in portrait mode.
2. Set the screen to be left of laptop screen.
3. Log out. Powerdown and restart. Wallpaper is landscape not portrait.
4. Log out and login (no powerdown). Sometimes works OK, sometimes not.

Actual Results:  
The bottom part of the portrait screen is black. The image has spilled over on
the the right hand monitor. You have to click on the righthand background to
get over the top of the lefthand background.

Expected Results:  
Left hand monitor should have the wallpaper in portrait mode (which it does
occasionally).

I've tried various combinations of moving the screen positions on the settings.
Disabling and re-enabling etc. etc. I can't find any particular thing that will
make it always work in portrait mode.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Bhushan Shah

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

Review request for Plasma and Martin Gräßlin.


Repository: plasma-workspace


Description
---

Advantage of this split is some parts such as showLockWindow and hideLockWindow 
needs platform specific code. So instead of doing complex if/else branching we 
can have seperate X11Locker and WaylandLocker.


Diffs
-

  ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
  ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
  ksmserver/screenlocker/CMakeLists.txt 24a89f6 
  ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
  ksmserver/screenlocker/ksldapp.h b281ab6 
  ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
  ksmserver/screenlocker/lockwindow.h c4013be 
  ksmserver/screenlocker/lockwindow.h c4013be 
  ksmserver/screenlocker/lockwindow.cpp e46733e 

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


Testing
---

builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
also works.


Thanks,

Bhushan Shah

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


Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Bhushan Shah

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

(Updated Oct. 26, 2015, 1:41 p.m.)


Review request for Plasma and Martin Gräßlin.


Changes
---

Fix issues, as well as revert move of lockwindow.{h,cpp} -> x11locker.{h,cpp} 
for ease in reviewing.


Repository: plasma-workspace


Description
---

Advantage of this split is some parts such as showLockWindow and hideLockWindow 
needs platform specific code. So instead of doing complex if/else branching we 
can have seperate X11Locker and WaylandLocker.


Diffs (updated)
-

  ksmserver/screenlocker/CMakeLists.txt 24a89f6 
  ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
  ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
  ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
  ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
  ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
  ksmserver/screenlocker/ksldapp.h b281ab6 
  ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
  ksmserver/screenlocker/lockwindow.h c4013be 
  ksmserver/screenlocker/lockwindow.cpp e46733e 

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


Testing
---

builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
also works.


Thanks,

Bhushan Shah

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


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread Marco Martin

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

Ship it!


i like it, almost good to go


applets/systemtray/package/contents/ui/ExpandedRepresentation.qml (line 122)


this shouldn't be necessary, as hiddenView should still be just hidden when 
root.expandedTask != null, and when contentHeight < height shouldn't be 
interactive and scrollbar should be hidden


- Marco Martin


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


Review Request 125805: Improve lockwindow test

2015-10-26 Thread Bhushan Shah

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

Review request for Plasma and Martin Gräßlin.


Repository: plasma-workspace


Description
---

Earlier we were creating dummy window but it was just black/transperant, so 
given that isBlack check would succeed even if lockwindow is not shown in some 
cases.


Diffs
-

  ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 

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


Testing
---

tests pass


Thanks,

Bhushan Shah

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


Re: Review Request 125793: [Task Manager] Add proper close button to tooltip

2015-10-26 Thread Martin Gräßlin


> On Oct. 26, 2015, 3:22 a.m., Andrew Lake wrote:
> > Proper close button is welcome. Generally looks good to me, though I do 
> > think the close button works better with the consistency of the normal 
> > background rather than the randomness of the thumbnail as a background. 
> > Other than that, thumbs up from me!

hey that's not fair. For years there was complaint that we cannot put the close 
button on the window. Now finally we made that possible and what? Move close 
button outside again.

/me grumbles about waisted time and that I don't want to invest the time to 
make that work on Wayland as it's really, really a difficult task.


- Martin


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


On Oct. 25, 2015, 10:42 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125793/
> ---
> 
> (Updated Oct. 25, 2015, 10:42 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 350705
> https://bugs.kde.org/show_bug.cgi?id=350705
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> I just found this patch I made a while ago. This turns the handmade close 
> button into a regular ToolButton (which is only shown when hovering) with a 
> proper size. Also, the button is always in the top right corner of the 
> *thumbnail* rather than the dialog
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/ToolTipDelegate.qml f7084c7 
> 
> Diff: https://git.reviewboard.kde.org/r/125793/diff/
> 
> 
> Testing
> ---
> 
> Works.
> 
> 
> File Attachments
> 
> 
> New close button
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/25/0fea55c5-5a9b-4321-bec5-34ec3bacf239__closebuttonproper.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Martin Gräßlin

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



ksmserver/screenlocker/abstractlocker.h (line 66)


stayOnTop is public in the Abstract class, but private in the implementing 
class. Is it supposed to be public? If not I suggest to move to protected.



ksmserver/screenlocker/x11locker.h (lines 47 - 48)


add "override" to the overriden methods


- Martin Gräßlin


On Oct. 26, 2015, 8:32 a.m., Bhushan Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125802/
> ---
> 
> (Updated Oct. 26, 2015, 8:32 a.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Advantage of this split is some parts such as showLockWindow and 
> hideLockWindow needs platform specific code. So instead of doing complex 
> if/else branching we can have seperate X11Locker and WaylandLocker.
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
>   ksmserver/screenlocker/CMakeLists.txt 24a89f6 
>   ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
>   ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
>   ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
>   ksmserver/screenlocker/ksldapp.h b281ab6 
>   ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
>   ksmserver/screenlocker/lockwindow.h c4013be 
>   ksmserver/screenlocker/lockwindow.cpp e46733e 
>   ksmserver/screenlocker/x11locker.h PRE-CREATION 
>   ksmserver/screenlocker/x11locker.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125802/diff/
> 
> 
> Testing
> ---
> 
> builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
> also works.
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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


Re: Review Request 125793: [Task Manager] Add proper close button to tooltip

2015-10-26 Thread Kai Uwe Broulik


> On Okt. 26, 2015, 2:22 vorm., Andrew Lake wrote:
> > Proper close button is welcome. Generally looks good to me, though I do 
> > think the close button works better with the consistency of the normal 
> > background rather than the randomness of the thumbnail as a background. 
> > Other than that, thumbs up from me!
> 
> Martin Gräßlin wrote:
> hey that's not fair. For years there was complaint that we cannot put the 
> close button on the window. Now finally we made that possible and what? Move 
> close button outside again.
> 
> /me grumbles about waisted time and that I don't want to invest the time 
> to make that work on Wayland as it's really, really a difficult task.

Also note that depending on the window geometry the thumbnail would get wider 
eventually partially or entirely leaking behind the close button. The partial 
case was what I wanted to avoid.


- Kai Uwe


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


On Okt. 25, 2015, 9:42 nachm., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125793/
> ---
> 
> (Updated Okt. 25, 2015, 9:42 nachm.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 350705
> https://bugs.kde.org/show_bug.cgi?id=350705
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> I just found this patch I made a while ago. This turns the handmade close 
> button into a regular ToolButton (which is only shown when hovering) with a 
> proper size. Also, the button is always in the top right corner of the 
> *thumbnail* rather than the dialog
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/ToolTipDelegate.qml f7084c7 
> 
> Diff: https://git.reviewboard.kde.org/r/125793/diff/
> 
> 
> Testing
> ---
> 
> Works.
> 
> 
> File Attachments
> 
> 
> New close button
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/25/0fea55c5-5a9b-4321-bec5-34ec3bacf239__closebuttonproper.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 125793: [Task Manager] Add proper close button to tooltip

2015-10-26 Thread Marco Martin


> On Oct. 26, 2015, 2:22 a.m., Andrew Lake wrote:
> > Proper close button is welcome. Generally looks good to me, though I do 
> > think the close button works better with the consistency of the normal 
> > background rather than the randomness of the thumbnail as a background. 
> > Other than that, thumbs up from me!
> 
> Martin Gräßlin wrote:
> hey that's not fair. For years there was complaint that we cannot put the 
> close button on the window. Now finally we made that possible and what? Move 
> close button outside again.
> 
> /me grumbles about waisted time and that I don't want to invest the time 
> to make that work on Wayland as it's really, really a difficult task.
> 
> Kai Uwe Broulik wrote:
> Also note that depending on the window geometry the thumbnail would get 
> wider eventually partially or entirely leaking behind the close button. The 
> partial case was what I wanted to avoid.

with the button outside the thumbnail, how would it look when there are more 
thumbnails in the tooltip due to grouped windows?


- Marco


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


On Oct. 25, 2015, 9:42 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125793/
> ---
> 
> (Updated Oct. 25, 2015, 9:42 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 350705
> https://bugs.kde.org/show_bug.cgi?id=350705
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> I just found this patch I made a while ago. This turns the handmade close 
> button into a regular ToolButton (which is only shown when hovering) with a 
> proper size. Also, the button is always in the top right corner of the 
> *thumbnail* rather than the dialog
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/ToolTipDelegate.qml f7084c7 
> 
> Diff: https://git.reviewboard.kde.org/r/125793/diff/
> 
> 
> Testing
> ---
> 
> Works.
> 
> 
> File Attachments
> 
> 
> New close button
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/25/0fea55c5-5a9b-4321-bec5-34ec3bacf239__closebuttonproper.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Review Request 125817: Add plugin system for Calendar events

2015-10-26 Thread Martin Klapetek

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

Review request for Plasma and Daniel Vrátil.


Repository: plasma-framework


Description
---

This adds a simple plugin interface that can be subclassed
and provide events integration with Plasma Calendar applet.

It's asynchronous and I've kept it deliberately simple.
For now the Calendar tells the plugins which date range
is being displayed, the plugins load the data and then
emit the dataReady() signal containing the events.

The events are stored in a multihash for quick access
by the Calendar's agenda part but also for overall
easy-to-use (eg. in teh model data()).

The event data is stored in EventData class, which has
a pretty self-explanatory members, except perhaps the
"isMinor" one. The intention with this is to support
namedays, where in some countries the calendars have
different name every day. This is just a minor holiday
and as such should not mark the calendar grid, otherwise
the whole grid would be in a different color.

Putting the interface here might raise the question of
depending on plasma-framework, but plugins provided by
KDE can go to plasma-workspace and other 3rd party ones
would just have to live with it. I don't think it will
be a problem but if it turns out it is, we can rethink
the placement.


Diffs
-

  src/declarativeimports/calendar/CMakeLists.txt 40ead91 
  src/declarativeimports/calendar/calendarplugin.cpp bafe80c 
  src/declarativeimports/calendar/daysmodel.h a5bdac9 
  src/declarativeimports/calendar/daysmodel.cpp 2d059a8 
  src/declarativeimports/calendar/eventdatadecorator.h PRE-CREATION 
  src/declarativeimports/calendar/eventdatadecorator.cpp PRE-CREATION 
  src/declarativeimports/calendar/plasmacalendarintegration/CMakeLists.txt 
PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/PlasmaCalendarIntegrationConfig.cmake.in
 PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.cpp
 PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/plasmacalendarintegration_export.h
 PRE-CREATION 

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


Testing
---

I have a simple KHolidays based plugin written (patch should be up later today)
and patches in the Calendar applet. Everything works as expected:
* the days are marked as containing an event
* the agenda part displays details of that event (name)


Thanks,

Martin Klapetek

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


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Gregor Mi


> On Oct. 25, 2015, 10:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.
> 
> Eike Hein wrote:
> Thomas, all this does is not grey out the text in Task Manager button 
> labels when a window is minimized. See also the long discussion in the 
> referenced bug ticket.
> 
> Thomas Pfeiffer wrote:
> Thanks, I just wanted to make sure what it specifically does.
> In that case, I am against the patch as it is. There should be some way 
> of distinguishing minimized from non-minimized tasks.
> I agree with the problem, but I do not agree with the solution.
> 
> Would it be possible to instead of hardcoded greying-out (which I also 
> agree is not the right representation, because greying out is reserved for 
> disabled controls) allow the Plasma theme to define the visual representation?
> In the Breeze theme, it would make sense (as suggested in the bug thread) 
> to use a different color for the highlight bar instead of changing the 
> representation of the task. Other themes might choose differen 
> representations, or just don't distinguish them at all.
> 
> Heiko Tietze wrote:
> Other solutions: size (e.g. make the button smaller), text (brackets for 
> instance), position (special area outside the normal task bar), subtile grey 
> out (icon only), or add some decorator to the icon. The color might work well 
> - and would probably be the easiest way, but I'm afraid of the desire to show 
> more states like maximized, docked, active, different virtual screen etc. 
> with the need to remember all the colors. Personally I'd like the position 
> very much since minimizing a window means to not include it into the 
> workflow, more or less. It's like the most missing feature of minimizing into 
> tray. However that would be quite a big change.
> 
> Eike Hein wrote:
> Changing size or position is jarring because it causes a lot of movement 
> on the bar. Desaturating the icon is unpopular with users too, since the 
> color information is desired to recognize an icon quickly.
> 
> Christoph Feck wrote:
> If I understand the bug report correctly, the issue was with the icon 
> only, so the text graying could be kept, and the icon graying removed.

See e.g. comment 33 "Faded text and colours makes this difficult". So it is 
also about the text.


- Gregor


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


On Oct. 23, 2015, 11:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 11:16 a.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml 
> f655801ab1f7b1a9a915f35149c858f0ede22a29 
> 
> Diff: https://git.reviewboard.kde.org/r/124675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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


Review Request 125820: [Desktop Toolbox] Keep hovered state while menu is open

2015-10-26 Thread Kai Uwe Broulik

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

Review request for Plasma and Andrew Lake.


Repository: plasma-desktop


Description
---

Before the toolbox button would stop glowing as soon as you interacted with 
menu items, leaving the button's mouse area. This makes the glow stay while the 
menu is opened to be consistent with other menus.


Diffs
-

  toolboxes/desktoptoolbox/contents/ui/ToolBoxButton.qml 9b3e84d 

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


Testing
---


File Attachments


Glow while menu open
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/10/26/fbc04712-6b62-4de6-af7b-18a1fad2ceff__toolboxhover.png


Thanks,

Kai Uwe Broulik

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


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Eike Hein


> On Oct. 25, 2015, 10:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.
> 
> Eike Hein wrote:
> Thomas, all this does is not grey out the text in Task Manager button 
> labels when a window is minimized. See also the long discussion in the 
> referenced bug ticket.
> 
> Thomas Pfeiffer wrote:
> Thanks, I just wanted to make sure what it specifically does.
> In that case, I am against the patch as it is. There should be some way 
> of distinguishing minimized from non-minimized tasks.
> I agree with the problem, but I do not agree with the solution.
> 
> Would it be possible to instead of hardcoded greying-out (which I also 
> agree is not the right representation, because greying out is reserved for 
> disabled controls) allow the Plasma theme to define the visual representation?
> In the Breeze theme, it would make sense (as suggested in the bug thread) 
> to use a different color for the highlight bar instead of changing the 
> representation of the task. Other themes might choose differen 
> representations, or just don't distinguish them at all.
> 
> Heiko Tietze wrote:
> Other solutions: size (e.g. make the button smaller), text (brackets for 
> instance), position (special area outside the normal task bar), subtile grey 
> out (icon only), or add some decorator to the icon. The color might work well 
> - and would probably be the easiest way, but I'm afraid of the desire to show 
> more states like maximized, docked, active, different virtual screen etc. 
> with the need to remember all the colors. Personally I'd like the position 
> very much since minimizing a window means to not include it into the 
> workflow, more or less. It's like the most missing feature of minimizing into 
> tray. However that would be quite a big change.

Changing size or position is jarring because it causes a lot of movement on the 
bar. Desaturating the icon is unpopular with users too, since the color 
information is desired to recognize an icon quickly.


- Eike


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


On Oct. 23, 2015, 11:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 11:16 a.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml 
> f655801ab1f7b1a9a915f35149c858f0ede22a29 
> 
> Diff: https://git.reviewboard.kde.org/r/124675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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


Re: Review Request 125817: Add plugin system for Calendar events

2015-10-26 Thread Martin Klapetek

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

(Updated Oct. 26, 2015, 9:22 p.m.)


Review request for Plasma and Daniel Vrátil.


Changes
---

Markdown being stupid.


Repository: plasma-framework


Description
---

This adds a simple plugin interface that can be subclassed
and provide events integration with Plasma Calendar applet.

It's asynchronous and I've kept it deliberately simple.
For now the Calendar tells the plugins which date range
is being displayed, the plugins load the data and then
emit the dataReady() signal containing the events.

The events are stored in a multihash for quick access
by the Calendar's agenda part but also for overall
easy-to-use (eg. in teh model data()).

The event data is stored in EventData class, which has
a pretty self-explanatory members, except perhaps the
"isMinor" one. The intention with this is to support
namedays, where in some countries the calendars have
different name every day. This is just a minor holiday
and as such should not mark the calendar grid, otherwise
the whole grid would be in a different color.

Putting the interface here might raise the question of
depending on plasma-framework, but plugins provided by
KDE can go to plasma-workspace and other 3rd party ones
would just have to live with it. I don't think it will
be a problem but if it turns out it is, we can rethink
the placement.


Diffs
-

  src/declarativeimports/calendar/CMakeLists.txt 40ead91 
  src/declarativeimports/calendar/calendarplugin.cpp bafe80c 
  src/declarativeimports/calendar/daysmodel.h a5bdac9 
  src/declarativeimports/calendar/daysmodel.cpp 2d059a8 
  src/declarativeimports/calendar/eventdatadecorator.h PRE-CREATION 
  src/declarativeimports/calendar/eventdatadecorator.cpp PRE-CREATION 
  src/declarativeimports/calendar/plasmacalendarintegration/CMakeLists.txt 
PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/PlasmaCalendarIntegrationConfig.cmake.in
 PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.cpp
 PRE-CREATION 
  
src/declarativeimports/calendar/plasmacalendarintegration/plasmacalendarintegration_export.h
 PRE-CREATION 

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


Testing (updated)
---

I have a simple KHolidays based plugin written (patch should be up later today)
and patches in the Calendar applet.

Everything works as expected:
* the days are marked as containing an event
* the agenda part displays details of that event (name)


Thanks,

Martin Klapetek

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


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Thomas Pfeiffer


> On Oct. 25, 2015, 10:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.
> 
> Eike Hein wrote:
> Thomas, all this does is not grey out the text in Task Manager button 
> labels when a window is minimized. See also the long discussion in the 
> referenced bug ticket.

Thanks, I just wanted to make sure what it specifically does.
In that case, I am against the patch as it is. There should be some way of 
distinguishing minimized from non-minimized tasks.
I agree with the problem, but I do not agree with the solution.

Would it be possible to instead of hardcoded greying-out (which I also agree is 
not the right representation, because greying out is reserved for disabled 
controls) allow the Plasma theme to define the visual representation?
In the Breeze theme, it would make sense (as suggested in the bug thread) to 
use a different color for the highlight bar instead of changing the 
representation of the task. Other themes might choose differen representations, 
or just don't distinguish them at all.


- Thomas


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


On Oct. 23, 2015, 11:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 11:16 a.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml 
> f655801ab1f7b1a9a915f35149c858f0ede22a29 
> 
> Diff: https://git.reviewboard.kde.org/r/124675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Heiko Tietze


> On Oct. 25, 2015, 10:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.
> 
> Eike Hein wrote:
> Thomas, all this does is not grey out the text in Task Manager button 
> labels when a window is minimized. See also the long discussion in the 
> referenced bug ticket.
> 
> Thomas Pfeiffer wrote:
> Thanks, I just wanted to make sure what it specifically does.
> In that case, I am against the patch as it is. There should be some way 
> of distinguishing minimized from non-minimized tasks.
> I agree with the problem, but I do not agree with the solution.
> 
> Would it be possible to instead of hardcoded greying-out (which I also 
> agree is not the right representation, because greying out is reserved for 
> disabled controls) allow the Plasma theme to define the visual representation?
> In the Breeze theme, it would make sense (as suggested in the bug thread) 
> to use a different color for the highlight bar instead of changing the 
> representation of the task. Other themes might choose differen 
> representations, or just don't distinguish them at all.

Other solutions: size (e.g. make the button smaller), text (brackets for 
instance), position (special area outside the normal task bar), subtile grey 
out (icon only), or add some decorator to the icon. The color might work well - 
and would probably be the easiest way, but I'm afraid of the desire to show 
more states like maximized, docked, active, different virtual screen etc. with 
the need to remember all the colors. Personally I'd like the position very much 
since minimizing a window means to not include it into the workflow, more or 
less. It's like the most missing feature of minimizing into tray. However that 
would be quite a big change.


- Heiko


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


On Oct. 23, 2015, 11:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 11:16 a.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml 
> f655801ab1f7b1a9a915f35149c858f0ede22a29 
> 
> Diff: https://git.reviewboard.kde.org/r/124675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread David Rosca


> On Oct. 26, 2015, 10:13 a.m., Sebastian Kügler wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 81
> > 
> >
> > Why are you removing this? It seems unrelated to the rest of this patch.
> 
> David Rosca wrote:
> Because I couldn't find any use of this and thus I don't know if it 
> should be in ListView or in ScrollArea (that now has "hiddenView" id). I 
> guess it was used from C++ to get handle of the ListView, but isn't used 
> anymore?
> 
> Sebastian Kügler wrote:
> Alright, I think it can be removed, but please make sure by grepping for 
> "hiddenView" through the source. If it's not used, feel free to drop this 
> issue.

I actually did just that before removing it :) There is no use of it in 
plasma-workspace.


- David


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


Re: Review Request 125820: [Desktop Toolbox] Keep hovered state while menu is open

2015-10-26 Thread Sebastian Kügler

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

Ship it!


Ship It!

- Sebastian Kügler


On Oct. 26, 2015, 9:17 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125820/
> ---
> 
> (Updated Oct. 26, 2015, 9:17 p.m.)
> 
> 
> Review request for Plasma and Andrew Lake.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Before the toolbox button would stop glowing as soon as you interacted with 
> menu items, leaving the button's mouse area. This makes the glow stay while 
> the menu is opened to be consistent with other menus.
> 
> 
> Diffs
> -
> 
>   toolboxes/desktoptoolbox/contents/ui/ToolBoxButton.qml 9b3e84d 
> 
> Diff: https://git.reviewboard.kde.org/r/125820/diff/
> 
> 
> Testing
> ---
> 
> 
> File Attachments
> 
> 
> Glow while menu open
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/26/fbc04712-6b62-4de6-af7b-18a1fad2ceff__toolboxhover.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread Sebastian Kügler


> On Oct. 26, 2015, 10:13 a.m., Sebastian Kügler wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 81
> > 
> >
> > Why are you removing this? It seems unrelated to the rest of this patch.
> 
> David Rosca wrote:
> Because I couldn't find any use of this and thus I don't know if it 
> should be in ListView or in ScrollArea (that now has "hiddenView" id). I 
> guess it was used from C++ to get handle of the ListView, but isn't used 
> anymore?

Alright, I think it can be removed, but please make sure by grepping for 
"hiddenView" through the source. If it's not used, feel free to drop this issue.


- Sebastian


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


Re: Review Request 125810: Add some formfactor info to plasma-workspace applets

2015-10-26 Thread Sebastian Kügler

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


Some remarks, but otherwise, thanks for taking care of this.


applets/devicenotifier/package/metadata.desktop (line 172)


Could you share the reasoning for this choice?



applets/notifications/package/metadata.desktop (line 170)


We have a special notifications applet for handsets in plasma-mobile, so 
this should probably be more limited (exactly the opposite keys of 
plasma-mobile's notifications applet).


- Sebastian Kügler


On Oct. 26, 2015, 4:26 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125810/
> ---
> 
> (Updated Oct. 26, 2015, 4:26 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> added some formfactor keywords in the applets metadata. once the explorer 
> model filters them accordingly, it should remove a bit of applets problematic 
> on the phone for one reason or another
> 
> 
> Diffs
> -
> 
>   applets/activitybar/metadata.desktop d6cf69c 
>   applets/analog-clock/metadata.desktop 4246c6e 
>   applets/batterymonitor/package/metadata.desktop a4ee702 
>   applets/calendar/metadata.desktop 5891264 
>   applets/clipboard/metadata.desktop 1d699d2 
>   applets/devicenotifier/package/metadata.desktop d71ce72 
>   applets/digital-clock/package/metadata.desktop b881453 
>   applets/lock_logout/metadata.desktop f53f541 
>   applets/mediacontroller/metadata.desktop d4ea37a 
>   applets/notifications/package/metadata.desktop 2beb20d 
>   applets/systemmonitor/cpu/metadata.desktop 76cd74d 
>   applets/systemmonitor/diskactivity/metadata.desktop 2a1ceaa 
>   applets/systemmonitor/diskusage/metadata.desktop 08344a8 
>   applets/systemmonitor/net/metadata.desktop 6671fba 
>   applets/systemtray/package/metadata.desktop 0ce0512 
> 
> Diff: https://git.reviewboard.kde.org/r/125810/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Christoph Feck


> On Oct. 25, 2015, 10:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.
> 
> Eike Hein wrote:
> Thomas, all this does is not grey out the text in Task Manager button 
> labels when a window is minimized. See also the long discussion in the 
> referenced bug ticket.
> 
> Thomas Pfeiffer wrote:
> Thanks, I just wanted to make sure what it specifically does.
> In that case, I am against the patch as it is. There should be some way 
> of distinguishing minimized from non-minimized tasks.
> I agree with the problem, but I do not agree with the solution.
> 
> Would it be possible to instead of hardcoded greying-out (which I also 
> agree is not the right representation, because greying out is reserved for 
> disabled controls) allow the Plasma theme to define the visual representation?
> In the Breeze theme, it would make sense (as suggested in the bug thread) 
> to use a different color for the highlight bar instead of changing the 
> representation of the task. Other themes might choose differen 
> representations, or just don't distinguish them at all.
> 
> Heiko Tietze wrote:
> Other solutions: size (e.g. make the button smaller), text (brackets for 
> instance), position (special area outside the normal task bar), subtile grey 
> out (icon only), or add some decorator to the icon. The color might work well 
> - and would probably be the easiest way, but I'm afraid of the desire to show 
> more states like maximized, docked, active, different virtual screen etc. 
> with the need to remember all the colors. Personally I'd like the position 
> very much since minimizing a window means to not include it into the 
> workflow, more or less. It's like the most missing feature of minimizing into 
> tray. However that would be quite a big change.
> 
> Eike Hein wrote:
> Changing size or position is jarring because it causes a lot of movement 
> on the bar. Desaturating the icon is unpopular with users too, since the 
> color information is desired to recognize an icon quickly.

If I understand the bug report correctly, the issue was with the icon only, so 
the text graying could be kept, and the icon graying removed.


- Christoph


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


On Oct. 23, 2015, 11:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 11:16 a.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml 
> f655801ab1f7b1a9a915f35149c858f0ede22a29 
> 
> Diff: https://git.reviewboard.kde.org/r/124675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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


Re: Review Request 125817: Add plugin system for Calendar events

2015-10-26 Thread Daniel Vrátil

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



src/declarativeimports/calendar/daysmodel.cpp (line 102)


const



src/declarativeimports/calendar/daysmodel.cpp (line 131)


const



src/declarativeimports/calendar/daysmodel.cpp (line 140)


Is this iteration necessary just to print debug output?

Once we have a PIM plugin with many events, this will get very very noisy.



src/declarativeimports/calendar/daysmodel.cpp (line 146)


Call `m_eventsData.reserve(m_eventsData.size() + data.size())` to reduce 
allocations (due to how QMultiHash::operator+= is implementated).



src/declarativeimports/calendar/daysmodel.cpp (line 161)


const



src/declarativeimports/calendar/daysmodel.cpp (line 162)


`m_qmlData.reserve(events.size())` to reduce allocations



src/declarativeimports/calendar/daysmodel.cpp (line 167)


This return statement probably shouldn't be there?



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 31)


IMO the members here should be hidden in PIMPL.

1) it would prevent ABI breakage (you can reoder members easilly when 
adding new ones)
2) it reduced copying and memory use when inserting/retrieving the objects 
from the QHash as well as passing a copy to EventDataDecorator.



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 40)


This means that all-day events that span over multiple days will be 
ignored, which breaks many common PIM events.

Otherwise the documentation should explictly say that the implementation 
has to resolve multi-day events into multiple EventData objects.



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 42)


You can move the bools to the end of the member list to save some bytes on 
padding.



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 44)


Should be called "description" IMO. "Comment" in the context of 
events/todos means something different (see RFC2445, part 4.8.1.4)



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 53)


explicit?



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h
 (line 54)


Dtor should be virtual



src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.cpp
 (line 25)


: QObject(parent)


- Daniel Vrátil


On Oct. 26, 2015, 9:22 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125817/
> ---
> 
> (Updated Oct. 26, 2015, 9:22 p.m.)
> 
> 
> Review request for Plasma and Daniel Vrátil.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This adds a simple plugin interface that can be subclassed
> and provide events integration with Plasma Calendar applet.
> 
> It's asynchronous and I've kept it deliberately simple.
> For now the Calendar tells the plugins which date range
> is being displayed, the plugins load the data and then
> emit the dataReady() signal containing the events.
> 
> The events are stored in a multihash for quick access
> by the Calendar's agenda part but also for overall
> easy-to-use (eg. in teh model data()).
> 
> The event data is stored in EventData class, which has
> a pretty self-explanatory members, except perhaps the
> "isMinor" one. The intention with this is to support
> namedays, where in some countries the calendars have
> different name every day. This is just a minor holiday
> and as such should not mark the calendar grid, otherwise
> the whole grid would be in a different color.
> 
> Putting the interface here might raise the question of
> depending on plasma-framework, but plugins provided by
> KDE can go to plasma-workspace and other 3rd party ones
> would just have to live with it. I don't 

Re: Review Request 125555: Port KAuth return code error checking in fontinst

2015-10-26 Thread David Edmundson

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

(Updated Oct. 26, 2015, 10:32 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 7ebe57aa74cd99ff81c4d0be9f1423a47b94d64b by David 
Edmundson to branch Plasma/5.4.


Repository: plasma-desktop


Description
---

Someone whilst porting had simply commented it out
This causes the install dialog to freeze if there's a problem.

CCBUG: 344473
CCBUG: 300951
CCBUG: 345234


Diffs
-

  kcms/kfontinst/dbus/FontInst.cpp 42413c1a05717f416681ef32317b8914c2ba35ec 

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


Testing
---


Thanks,

David Edmundson

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


Re: Review Request 125820: [Desktop Toolbox] Keep hovered state while menu is open

2015-10-26 Thread Kai Uwe Broulik

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

(Updated Oct. 26, 2015, 10:31 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma and Andrew Lake.


Changes
---

Submitted with commit 45481bb57184fcd4b28adf9f741093ecbc281786 by Kai Uwe 
Broulik to branch Plasma/5.4.


Repository: plasma-desktop


Description
---

Before the toolbox button would stop glowing as soon as you interacted with 
menu items, leaving the button's mouse area. This makes the glow stay while the 
menu is opened to be consistent with other menus.


Diffs
-

  toolboxes/desktoptoolbox/contents/ui/ToolBoxButton.qml 9b3e84d 

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


Testing
---


File Attachments


Glow while menu open
  
https://git.reviewboard.kde.org/media/uploaded/files/2015/10/26/fbc04712-6b62-4de6-af7b-18a1fad2ceff__toolboxhover.png


Thanks,

Kai Uwe Broulik

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


Re: Review Request 125822: [Kickoff] Support mouse back button in ApplicationsView

2015-10-26 Thread Kai Uwe Broulik

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

(Updated Oct. 26, 2015, 10:48 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 23919d4f0552a4c7b361cbcf768c6d65d2174e8d by Kai Uwe 
Broulik to branch Plasma/5.4.


Repository: plasma-desktop


Description
---

Clicking the back mouse button while hovering the applications list will go up 
one level.

CCBUG: 353971


Diffs
-

  applets/kickoff/package/contents/ui/ApplicationsView.qml 96589c4 

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


Testing
---

Went to Games -> Arcade, pressed back buton, ended up in "Games", pressed back 
again, ended up in the applications overview


Thanks,

Kai Uwe Broulik

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


Re: Review Request 125810: Add some formfactor info to plasma-workspace applets

2015-10-26 Thread Sebastian Kügler


> On Oct. 26, 2015, 5:26 p.m., Kai Uwe Broulik wrote:
> > applets/analog-clock/metadata.desktop, line 170
> > 
> >
> > Why not "not specified = everywhere"?
> 
> Marco Martin wrote:
> yeah, in some of those are probably redundant

Yes, the formfactor stuff is meant as a specialization, if you specify them 
all, might as well leave them out. I've specified that in the examples on my 
blog as well: http://vizzzion.org/blog/2015/07/convergence-through-divergence/


- Sebastian


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


On Oct. 26, 2015, 4:26 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125810/
> ---
> 
> (Updated Oct. 26, 2015, 4:26 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> added some formfactor keywords in the applets metadata. once the explorer 
> model filters them accordingly, it should remove a bit of applets problematic 
> on the phone for one reason or another
> 
> 
> Diffs
> -
> 
>   applets/activitybar/metadata.desktop d6cf69c 
>   applets/analog-clock/metadata.desktop 4246c6e 
>   applets/batterymonitor/package/metadata.desktop a4ee702 
>   applets/calendar/metadata.desktop 5891264 
>   applets/clipboard/metadata.desktop 1d699d2 
>   applets/devicenotifier/package/metadata.desktop d71ce72 
>   applets/digital-clock/package/metadata.desktop b881453 
>   applets/lock_logout/metadata.desktop f53f541 
>   applets/mediacontroller/metadata.desktop d4ea37a 
>   applets/notifications/package/metadata.desktop 2beb20d 
>   applets/systemmonitor/cpu/metadata.desktop 76cd74d 
>   applets/systemmonitor/diskactivity/metadata.desktop 2a1ceaa 
>   applets/systemmonitor/diskusage/metadata.desktop 08344a8 
>   applets/systemmonitor/net/metadata.desktop 6671fba 
>   applets/systemtray/package/metadata.desktop 0ce0512 
> 
> Diff: https://git.reviewboard.kde.org/r/125810/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Re: Review Request 125793: [Task Manager] Add proper close button to tooltip

2015-10-26 Thread Andrew Lake


> On Oct. 26, 2015, 2:22 a.m., Andrew Lake wrote:
> > Proper close button is welcome. Generally looks good to me, though I do 
> > think the close button works better with the consistency of the normal 
> > background rather than the randomness of the thumbnail as a background. 
> > Other than that, thumbs up from me!
> 
> Martin Gräßlin wrote:
> hey that's not fair. For years there was complaint that we cannot put the 
> close button on the window. Now finally we made that possible and what? Move 
> close button outside again.
> 
> /me grumbles about waisted time and that I don't want to invest the time 
> to make that work on Wayland as it's really, really a difficult task.
> 
> Kai Uwe Broulik wrote:
> Also note that depending on the window geometry the thumbnail would get 
> wider eventually partially or entirely leaking behind the close button. The 
> partial case was what I wanted to avoid.
> 
> Marco Martin wrote:
> with the button outside the thumbnail, how would it look when there are 
> more thumbnails in the tooltip due to grouped windows?

I'm not sure I remember which complaint was "we cannot put the close button on 
the window" and what discussion resulted in putting it on the window, but sorry 
if my observation resulted betrays some inconsistency. If we want to leave the 
close button on the the thumbnail itself, then all I'd suggest is that we 
provide some kind of consistent background for the thing to sit on. As it is, I 
think the noise of the thumbnail background makes the button difficult to 
distinguish. Maybe moving it outside is not the right solution for that. And 
maybe that suggestion conflicts with something we already provided guidance on. 
I'd simply request we please assume any inconsistency in what I've said is 
because of my awful memory, not because I'm running some consistency-checker 
against what I previously said to screw over the hard work anyone has done.


- Andrew


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


On Oct. 25, 2015, 9:42 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125793/
> ---
> 
> (Updated Oct. 25, 2015, 9:42 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Andrew Lake.
> 
> 
> Bugs: 350705
> https://bugs.kde.org/show_bug.cgi?id=350705
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> I just found this patch I made a while ago. This turns the handmade close 
> button into a regular ToolButton (which is only shown when hovering) with a 
> proper size. Also, the button is always in the top right corner of the 
> *thumbnail* rather than the dialog
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/ToolTipDelegate.qml f7084c7 
> 
> Diff: https://git.reviewboard.kde.org/r/125793/diff/
> 
> 
> Testing
> ---
> 
> Works.
> 
> 
> File Attachments
> 
> 
> New close button
>   
> https://git.reviewboard.kde.org/media/uploaded/files/2015/10/25/0fea55c5-5a9b-4321-bec5-34ec3bacf239__closebuttonproper.png
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Re: Review Request 125822: [Kickoff] Support mouse back button in ApplicationsView

2015-10-26 Thread David Edmundson

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

Ship it!


why only CC bug?

- David Edmundson


On Oct. 26, 2015, 10:41 p.m., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125822/
> ---
> 
> (Updated Oct. 26, 2015, 10:41 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Clicking the back mouse button while hovering the applications list will go 
> up one level.
> 
> CCBUG: 353971
> 
> 
> Diffs
> -
> 
>   applets/kickoff/package/contents/ui/ApplicationsView.qml 96589c4 
> 
> Diff: https://git.reviewboard.kde.org/r/125822/diff/
> 
> 
> Testing
> ---
> 
> Went to Games -> Arcade, pressed back buton, ended up in "Games", pressed 
> back again, ended up in the applications overview
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


Review Request 125822: [Kickoff] Support mouse back button in ApplicationsView

2015-10-26 Thread Kai Uwe Broulik

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

Review request for Plasma.


Repository: plasma-desktop


Description
---

Clicking the back mouse button while hovering the applications list will go up 
one level.

CCBUG: 353971


Diffs
-

  applets/kickoff/package/contents/ui/ApplicationsView.qml 96589c4 

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


Testing
---

Went to Games -> Arcade, pressed back buton, ended up in "Games", pressed back 
again, ended up in the applications overview


Thanks,

Kai Uwe Broulik

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


[Plasma Workspace Wallpapers] [Bug 354379] Wallpaper fails to rotate 90 on dual screen with external scr. on left. Stays horiz. and spills over onto right scr.

2015-10-26 Thread PhillB via KDE Bugzilla
https://bugs.kde.org/show_bug.cgi?id=354379

--- Comment #3 from PhillB  ---
This is only thing I've managed to find so far.

https://www.reddit.com/r/kde/comments/3cc570/displaymonitor_settings_reset_on_every_reboot/

I'm about to try the work around suggested in the last post which is:

On Fedora 22 my process was

rm ~/.local/share/kscreen/*
rm ~/.config/kactivitymanagerdc
rm ~/.config/plasma-org.kde.plasma.desktop-appletsrc
reboot
make configuration changes that i needed in kscreen, with background
settings, and added panels/widgets.
chmod 444 ~/.local/share/kscreen/*

Now it seems to be sticking for the most part. Sometimes when I log out and
then back in quickly it will revert but then I just reboot and everything is
how it should be. Weird.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


[Plasma Workspace Wallpapers] [Bug 354379] Wallpaper fails to rotate 90 on dual screen with external scr. on left. Stays horiz. and spills over onto right scr.

2015-10-26 Thread PhillB via KDE Bugzilla
https://bugs.kde.org/show_bug.cgi?id=354379

--- Comment #4 from PhillB  ---
I also deleted everything in ~/.kde/share/apps/kscreen/*.

Everything appears to be going OK for now. My backgrounds are portrait again
:-)

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread David Rosca

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

(Updated Oct. 26, 2015, 10:43 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit a2a9d41b334db36238232be8e093deeecab03514 by David Rosca 
to branch master.


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


Repository: plasma-workspace


Description
---

Show vertical scrollbar when hidden items doesn't fit in popup.


Diffs
-

  applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 

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


Testing
---

Works, although mouse scrolling works only on the scrollbar.


Thanks,

David Rosca

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


Re: Review Request 125822: [Kickoff] Support mouse back button in ApplicationsView

2015-10-26 Thread Kai Uwe Broulik


> On Okt. 26, 2015, 10:43 nachm., David Edmundson wrote:
> > why only CC bug?

Dunno, that guy mentioned something about going forward which this patch 
doesn't. But I'm fine with closing the bug nonetheless.


- Kai Uwe


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


On Okt. 26, 2015, 10:41 nachm., Kai Uwe Broulik wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125822/
> ---
> 
> (Updated Okt. 26, 2015, 10:41 nachm.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> Clicking the back mouse button while hovering the applications list will go 
> up one level.
> 
> CCBUG: 353971
> 
> 
> Diffs
> -
> 
>   applets/kickoff/package/contents/ui/ApplicationsView.qml 96589c4 
> 
> Diff: https://git.reviewboard.kde.org/r/125822/diff/
> 
> 
> Testing
> ---
> 
> Went to Games -> Arcade, pressed back buton, ended up in "Games", pressed 
> back again, ended up in the applications overview
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


[Plasma Workspace Wallpapers] [Bug 354379] Wallpaper fails to rotate 90 on dual screen with external scr. on left. Stays horiz. and spills over onto right scr.

2015-10-26 Thread PhillB via KDE Bugzilla
https://bugs.kde.org/show_bug.cgi?id=354379

--- Comment #6 from PhillB  ---
I have discovered that the background can be made to go portrait again by using
xrandr from the commandline.

Unplug the screen. Run xrandr (no parameters). This gets back to single screen
mode. Then plug the monitor back in and run xrandr again. Voila!

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Martin Klapetek


> On Oct. 25, 2015, 11:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.
> 
> Eike Hein wrote:
> Thomas, all this does is not grey out the text in Task Manager button 
> labels when a window is minimized. See also the long discussion in the 
> referenced bug ticket.
> 
> Thomas Pfeiffer wrote:
> Thanks, I just wanted to make sure what it specifically does.
> In that case, I am against the patch as it is. There should be some way 
> of distinguishing minimized from non-minimized tasks.
> I agree with the problem, but I do not agree with the solution.
> 
> Would it be possible to instead of hardcoded greying-out (which I also 
> agree is not the right representation, because greying out is reserved for 
> disabled controls) allow the Plasma theme to define the visual representation?
> In the Breeze theme, it would make sense (as suggested in the bug thread) 
> to use a different color for the highlight bar instead of changing the 
> representation of the task. Other themes might choose differen 
> representations, or just don't distinguish them at all.
> 
> Heiko Tietze wrote:
> Other solutions: size (e.g. make the button smaller), text (brackets for 
> instance), position (special area outside the normal task bar), subtile grey 
> out (icon only), or add some decorator to the icon. The color might work well 
> - and would probably be the easiest way, but I'm afraid of the desire to show 
> more states like maximized, docked, active, different virtual screen etc. 
> with the need to remember all the colors. Personally I'd like the position 
> very much since minimizing a window means to not include it into the 
> workflow, more or less. It's like the most missing feature of minimizing into 
> tray. However that would be quite a big change.
> 
> Eike Hein wrote:
> Changing size or position is jarring because it causes a lot of movement 
> on the bar. Desaturating the icon is unpopular with users too, since the 
> color information is desired to recognize an icon quickly.
> 
> Christoph Feck wrote:
> If I understand the bug report correctly, the issue was with the icon 
> only, so the text graying could be kept, and the icon graying removed.
> 
> Gregor Mi wrote:
> See e.g. comment 33 "Faded text and colours makes this difficult". So it 
> is also about the text.

Fwiw, I, as an ordinary user, do not see any difference between the window 
being minimalized and the window not being on top (active window). My laptop 
screen is small~ish (13") and I always work in maximized windows. Therefore, 
representing the minimized state in the taskbar in any way bears absolutely no 
value for me, because if I want to swtich to any other window, it makes no 
difference if it just moves in the stack of visible windows or if the window is 
unminimized. The goal is to get the window to the top/make it an active window, 
I do not care where it comes from. Or, more importantly, I do not need to care.

Have we considered a usecase like that? I kind of question the whole usefulness 
of representing the minimized state, switching to it makes no difference 
besides the unminimizing animation, it just makes it harder to find. I mean, 
I've never looked at the taskbar searching Konsole and thought to myself "ah 
right, Konsole is now minimized, so now what?". What would our personas do? Why 
is it so important to be able to tell if the window is minimized or not?


- Martin


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


On Oct. 23, 2015, 1:16 p.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 1:16 p.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   applets/taskmanager/package/contents/ui/Task.qml 
> f655801ab1f7b1a9a915f35149c858f0ede22a29 
> 
> Diff: https://git.reviewboard.kde.org/r/124675/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Gregor Mi
> 
>

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


Re: Review Request 124675: Fix Bug 311991 - Taskbar buttons for minimized apps should not use disabled state

2015-10-26 Thread Eli MacKenzie


> On Oct. 25, 2015, 6:05 p.m., Thomas Pfeiffer wrote:
> > As always: If you want design or usability input, please provide a 
> > screenshot. We cannot read the code. Thanks.
> 
> Eike Hein wrote:
> Thomas, all this does is not grey out the text in Task Manager button 
> labels when a window is minimized. See also the long discussion in the 
> referenced bug ticket.
> 
> Thomas Pfeiffer wrote:
> Thanks, I just wanted to make sure what it specifically does.
> In that case, I am against the patch as it is. There should be some way 
> of distinguishing minimized from non-minimized tasks.
> I agree with the problem, but I do not agree with the solution.
> 
> Would it be possible to instead of hardcoded greying-out (which I also 
> agree is not the right representation, because greying out is reserved for 
> disabled controls) allow the Plasma theme to define the visual representation?
> In the Breeze theme, it would make sense (as suggested in the bug thread) 
> to use a different color for the highlight bar instead of changing the 
> representation of the task. Other themes might choose differen 
> representations, or just don't distinguish them at all.
> 
> Heiko Tietze wrote:
> Other solutions: size (e.g. make the button smaller), text (brackets for 
> instance), position (special area outside the normal task bar), subtile grey 
> out (icon only), or add some decorator to the icon. The color might work well 
> - and would probably be the easiest way, but I'm afraid of the desire to show 
> more states like maximized, docked, active, different virtual screen etc. 
> with the need to remember all the colors. Personally I'd like the position 
> very much since minimizing a window means to not include it into the 
> workflow, more or less. It's like the most missing feature of minimizing into 
> tray. However that would be quite a big change.
> 
> Eike Hein wrote:
> Changing size or position is jarring because it causes a lot of movement 
> on the bar. Desaturating the icon is unpopular with users too, since the 
> color information is desired to recognize an icon quickly.
> 
> Christoph Feck wrote:
> If I understand the bug report correctly, the issue was with the icon 
> only, so the text graying could be kept, and the icon graying removed.
> 
> Gregor Mi wrote:
> See e.g. comment 33 "Faded text and colours makes this difficult". So it 
> is also about the text.
> 
> Martin Klapetek wrote:
> Fwiw, I, as an ordinary user, do not see any difference between the 
> window being minimalized and the window not being on top (active window). My 
> laptop screen is small~ish (13") and I always work in maximized windows. 
> Therefore, representing the minimized state in the taskbar in any way bears 
> absolutely no value for me, because if I want to swtich to any other window, 
> it makes no difference if it just moves in the stack of visible windows or if 
> the window is unminimized. The goal is to get the window to the top/make it 
> an active window, I do not care where it comes from. Or, more importantly, I 
> do not need to care.
> 
> Have we considered a usecase like that? I kind of question the whole 
> usefulness of representing the minimized state, switching to it makes no 
> difference besides the unminimizing animation, it just makes it harder to 
> find. I mean, I've never looked at the taskbar searching Konsole and thought 
> to myself "ah right, Konsole is now minimized, so now what?". What would our 
> personas do? Why is it so important to be able to tell if the window is 
> minimized or not?

My own use-case for a visible minimized state is as a flag. As far as I know 
there is no means to associate arbitrary data to a taskbar entry, so I'll 
minimize a window when it is "flagged". For example, if a browser window stays 
minimized "too long" I'll know I have to go bookmark some of its tabs and close 
it. I tend to remain logged in for months at a time.

FWIW I have a 24" 16:10 monitor and use maximized windows.


- Eli


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


On Oct. 23, 2015, 7:16 a.m., Gregor Mi wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124675/
> ---
> 
> (Updated Oct. 23, 2015, 7:16 a.m.)
> 
> 
> Review request for Plasma, KDE Usability and Jens Reuterberg.
> 
> 
> Bugs: 311991
> https://bugs.kde.org/show_bug.cgi?id=311991
> 
> 
> Repository: plasma-desktop
> 
> 
> Description
> ---
> 
> This fixes Bug 311991. Though I am not sure if there are side effects which I 
> am not aware of.
> 
> 
> Diffs
> -
> 
>   

[Plasma Workspace Wallpapers] [Bug 354379] Wallpaper fails to rotate 90 on dual screen with external scr. on left. Stays horiz. and spills over onto right scr.

2015-10-26 Thread PhillB via KDE Bugzilla
https://bugs.kde.org/show_bug.cgi?id=354379

--- Comment #5 from PhillB  ---
It's back again. Don't know what I did. Background always landscape always now.

To get it back to portrait I have to unplug the screen. Do all the deletes then
reboot. Then I plug the screen back in and run KScreen which sees the (re)added
screen and it works.

Wierd.

-- 
You are receiving this mail because:
You are the assignee for the bug.
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 125817: Add plugin system for Calendar events

2015-10-26 Thread Martin Klapetek


> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote:
> > src/declarativeimports/calendar/daysmodel.cpp, line 150
> > 
> >
> > Is this iteration necessary just to print debug output?
> > 
> > Once we have a PIM plugin with many events, this will get very very 
> > noisy.

Yeah, this shouldn't be there at all, I was just using it for debug purposes.


> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote:
> > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h,
> >  line 40
> > 
> >
> > This means that all-day events that span over multiple days will be 
> > ignored, which breaks many common PIM events.
> > 
> > Otherwise the documentation should explictly say that the 
> > implementation has to resolve multi-day events into multiple EventData 
> > objects.

It should be fine, the endTime can be three days ahead and isAllDay can be set 
to false by the plugin, then it will work. I added the isAllDay for easier 
working with the holidays which always have just a single date, so startDate + 
isAllDay = true, and that spares the model to check if it is from midnight to 
midnight.

But thinking about it, it could be revisited. I think I'll just remove it and 
will treat EventType == Holiday as if isAllDay==true.


> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote:
> > src/declarativeimports/calendar/daysmodel.cpp, line 177
> > 
> >
> > This return statement probably shouldn't be there?

No, wrong copypasta.


> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote:
> > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h,
> >  line 31
> > 
> >
> > IMO the members here should be hidden in PIMPL.
> > 
> > 1) it would prevent ABI breakage (you can reoder members easilly when 
> > adding new ones)
> > 2) it reduced copying and memory use when inserting/retrieving the 
> > objects from the QHash as well as passing a copy to EventDataDecorator.

Hm, I'll see about that. On the other hand - how many other things would we 
need to add in the future? If we can think of some more usecases not covered by 
these, please share it.


> On Oct. 26, 2015, 11:05 p.m., Daniel Vrátil wrote:
> > src/declarativeimports/calendar/plasmacalendarintegration/calendareventsplugin.h,
> >  line 44
> > 
> >
> > Should be called "description" IMO. "Comment" in the context of 
> > events/todos means something different (see RFC2445, part 4.8.1.4)

Good point.


- Martin


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


On Oct. 26, 2015, 9:22 p.m., Martin Klapetek wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125817/
> ---
> 
> (Updated Oct. 26, 2015, 9:22 p.m.)
> 
> 
> Review request for Plasma and Daniel Vrátil.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> This adds a simple plugin interface that can be subclassed
> and provide events integration with Plasma Calendar applet.
> 
> It's asynchronous and I've kept it deliberately simple.
> For now the Calendar tells the plugins which date range
> is being displayed, the plugins load the data and then
> emit the dataReady() signal containing the events.
> 
> The events are stored in a multihash for quick access
> by the Calendar's agenda part but also for overall
> easy-to-use (eg. in teh model data()).
> 
> The event data is stored in EventData class, which has
> a pretty self-explanatory members, except perhaps the
> "isMinor" one. The intention with this is to support
> namedays, where in some countries the calendars have
> different name every day. This is just a minor holiday
> and as such should not mark the calendar grid, otherwise
> the whole grid would be in a different color.
> 
> Putting the interface here might raise the question of
> depending on plasma-framework, but plugins provided by
> KDE can go to plasma-workspace and other 3rd party ones
> would just have to live with it. I don't think it will
> be a problem but if it turns out it is, we can rethink
> the placement.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/calendar/CMakeLists.txt 40ead91 
>   src/declarativeimports/calendar/calendarplugin.cpp bafe80c 
>   src/declarativeimports/calendar/daysmodel.h a5bdac9 
>   

Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread David Edmundson

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

Ship it!


ship it!

I include a grumble about something that existed before this patch, so you can 
consier it later (or ignore it)


ksmserver/screenlocker/abstractlocker.h (line 68)


I don't like this set.

It means if you use globalAccel() from X11Locker's constructor, you crash.

We currently don't, but it's leaving the safety off for the next person who 
edits this file.

I would either:
 - make a public accessor in KSLApp (which owns this object) and port code 
to  KSLDApp::self()->globalAccel(), getting rid m_globalAccel here.

 - pass it in the constructor to AbstractLocker


- David Edmundson


On Oct. 26, 2015, 11:37 a.m., Bhushan Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125802/
> ---
> 
> (Updated Oct. 26, 2015, 11:37 a.m.)
> 
> 
> Review request for Plasma, David Edmundson and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Advantage of this split is some parts such as showLockWindow and 
> hideLockWindow needs platform specific code. So instead of doing complex 
> if/else branching we can have seperate X11Locker and WaylandLocker.
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/CMakeLists.txt 24a89f6 
>   ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
>   ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
>   ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
>   ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
>   ksmserver/screenlocker/ksldapp.h b281ab6 
>   ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
>   ksmserver/screenlocker/lockwindow.h c4013be 
>   ksmserver/screenlocker/lockwindow.cpp e46733e 
> 
> Diff: https://git.reviewboard.kde.org/r/125802/diff/
> 
> 
> Testing
> ---
> 
> builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
> also works.
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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


Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Bhushan Shah


> On Oct. 26, 2015, 5:32 p.m., David Edmundson wrote:
> > ksmserver/screenlocker/abstractlocker.h, line 68
> > 
> >
> > I don't like this set.
> > 
> > It means if you use globalAccel() from X11Locker's constructor, you 
> > crash.
> > 
> > We currently don't, but it's leaving the safety off for the next person 
> > who edits this file.
> > 
> > 
> > I would either:
> >  - make a public accessor in KSLApp (which owns this object) and port 
> > code to  KSLDApp::self()->globalAccel(), getting rid m_globalAccel here.
> > 
> >  - pass it in the constructor to AbstractLocker

I will do it later in seperate review request.. currently I've 2-3 other 
patches depending upon this one and rebasing them would be mess I fear..


- Bhushan


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


On Oct. 26, 2015, 5:07 p.m., Bhushan Shah wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125802/
> ---
> 
> (Updated Oct. 26, 2015, 5:07 p.m.)
> 
> 
> Review request for Plasma, David Edmundson and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Advantage of this split is some parts such as showLockWindow and 
> hideLockWindow needs platform specific code. So instead of doing complex 
> if/else branching we can have seperate X11Locker and WaylandLocker.
> 
> 
> Diffs
> -
> 
>   ksmserver/screenlocker/CMakeLists.txt 24a89f6 
>   ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
>   ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
>   ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
>   ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
>   ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
>   ksmserver/screenlocker/ksldapp.h b281ab6 
>   ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
>   ksmserver/screenlocker/lockwindow.h c4013be 
>   ksmserver/screenlocker/lockwindow.cpp e46733e 
> 
> Diff: https://git.reviewboard.kde.org/r/125802/diff/
> 
> 
> Testing
> ---
> 
> builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
> also works.
> 
> 
> Thanks,
> 
> Bhushan Shah
> 
>

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


Jenkins-kde-ci: plasma-workspace master kf5-qt5 » Linux,gcc - Build # 230 - Failure!

2015-10-26 Thread no-reply

GENERAL INFO

BUILD FAILURE
Build URL: 
https://build.kde.org/job/plasma-workspace%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/230/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 26 Oct 2015 13:37:11 +
Build duration: 4 min 15 sec

CHANGE SET
Revision 5a0ab38613763ab50a0e1315615163a48fdb753b by Bhushan Shah: 
([screenlocker] Split generic parts of X11Locker into AbstractLocker)
  change: edit ksmserver/screenlocker/lockwindow.h
  change: delete ksmserver/screenlocker/autotests/lockwindowtest.cpp
  change: edit ksmserver/screenlocker/lockwindow.cpp
  change: edit ksmserver/screenlocker/autotests/CMakeLists.txt
  change: edit ksmserver/screenlocker/CMakeLists.txt
  change: edit ksmserver/screenlocker/ksldapp.cpp
  change: edit ksmserver/screenlocker/ksldapp.h
___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Re: Review Request 125802: Split generic parts from X11Locker into AbstractLocker

2015-10-26 Thread Bhushan Shah

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

(Updated Oct. 26, 2015, 1:36 p.m.)


Status
--

This change has been marked as submitted.


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


Changes
---

Submitted with commit 5a0ab38613763ab50a0e1315615163a48fdb753b by Bhushan Shah 
to branch master.


Repository: plasma-workspace


Description
---

Advantage of this split is some parts such as showLockWindow and hideLockWindow 
needs platform specific code. So instead of doing complex if/else branching we 
can have seperate X11Locker and WaylandLocker.


Diffs
-

  ksmserver/screenlocker/CMakeLists.txt 24a89f6 
  ksmserver/screenlocker/abstractlocker.h PRE-CREATION 
  ksmserver/screenlocker/abstractlocker.cpp PRE-CREATION 
  ksmserver/screenlocker/autotests/CMakeLists.txt ab901ca 
  ksmserver/screenlocker/autotests/lockwindowtest.cpp 444c44f 
  ksmserver/screenlocker/autotests/x11lockertest.cpp PRE-CREATION 
  ksmserver/screenlocker/ksldapp.h b281ab6 
  ksmserver/screenlocker/ksldapp.cpp 68ebfc2 
  ksmserver/screenlocker/lockwindow.h c4013be 
  ksmserver/screenlocker/lockwindow.cpp e46733e 

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


Testing
---

builds, installs, autotests and kscreenlocker_test passes, normal lockscreen 
also works.


Thanks,

Bhushan Shah

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


Re: Review Request 124589: Add Disk Quota Plasmoid

2015-10-26 Thread Dominik Haumann


> On Oct. 23, 2015, 10:15 p.m., David Edmundson wrote:
> > Bump? We have a "no huge merges freeze" on the 5th of November for 5.5

Thanks for the bump, I will have some time next weekend, will go over the 
reviews again and then push this if all is good. (Due to moving, I have no 
internet right now, so a bit inconvenient situation atm).


- Dominik


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


On Aug. 3, 2015, 5:34 p.m., Dominik Haumann wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124589/
> ---
> 
> (Updated Aug. 3, 2015, 5:34 p.m.)
> 
> 
> Review request for Plasma, Kai Uwe Broulik and Sebastian Kügler.
> 
> 
> Repository: kdeplasma-addons
> 
> 
> Description
> ---
> 
> The disk quota is usually used in enterprise installations where network 
> shares are mounted locally. Typically, sysadmins want to avoid that users 
> copy lots of data into their folders, and therefor set quotas (the quota 
> limit has nothing to do with the physical size of a partition). Typically, 
> once a user gets over the hard limit of the quota, the account is blocked and 
> the user cannot login anymore. This happens from time to time, since the 
> users are not really aware of the current quota limit and the already used 
> disk space.
> 
> Here is where the "Disk Quota" plasmoid helps: It continusouly monitors the 
> disk quota and warns the quota apprpriately.
> 
> A detailed description including screenshots can be found in this blog: 
> http://kate-editor.org/?p=3591
> 
> (I had a KDE4 hack of this plasmoid running at university, and it proved very 
> usable over the years, so it is probably a good idea to have it by default in 
> plasma)
> 
> Issues:
> - the panel icon is larger than the others (some wrong margin?)
> - an icon for the metadata.desktop is missing (the shipped quota.svg file is 
> not available here, it seems).
> - the grid units probably need some more tuning
> 
> 
> Diffs
> -
> 
>   applets/CMakeLists.txt c60c350 
>   applets/diskquota/CMakeLists.txt PRE-CREATION 
>   applets/diskquota/Messages.sh PRE-CREATION 
>   applets/diskquota/icons/quota.svg PRE-CREATION 
>   applets/diskquota/package/contents/ui/ListDelegateItem.qml PRE-CREATION 
>   applets/diskquota/package/contents/ui/main.qml PRE-CREATION 
>   applets/diskquota/package/metadata.desktop PRE-CREATION 
>   applets/diskquota/plugin/DiskQuota.h PRE-CREATION 
>   applets/diskquota/plugin/DiskQuota.cpp PRE-CREATION 
>   applets/diskquota/plugin/QuotaItem.h PRE-CREATION 
>   applets/diskquota/plugin/QuotaItem.cpp PRE-CREATION 
>   applets/diskquota/plugin/QuotaListModel.h PRE-CREATION 
>   applets/diskquota/plugin/QuotaListModel.cpp PRE-CREATION 
>   applets/diskquota/plugin/plugin.h PRE-CREATION 
>   applets/diskquota/plugin/plugin.cpp PRE-CREATION 
>   applets/diskquota/plugin/qmldir PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124589/diff/
> 
> 
> Testing
> ---
> 
> Tested combinations:
> - no quota installed: A nice message is displayed telling the user that 
> 'quota' is missing.
> - quota installed, but no quota restrictions set: The applet says "No quota 
> restrictions found"
> - quota installed, quotas active: The applet continuously shows the data. The 
> quota entries are in a QAbstractItemModel derived class, so 
> inserting/removing quotas all works (tested).
> - filelight installed: the item under mouse gets highlighted. If clicked, 
> filelight starts with the correct location.
> 
> 
> Thanks,
> 
> Dominik Haumann
> 
>

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


Jenkins-kde-ci: plasma-workspace master kf5-qt5 » Linux,gcc - Build # 231 - Fixed!

2015-10-26 Thread no-reply

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/plasma-workspace%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/231/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 26 Oct 2015 13:43:21 +
Build duration: 9 min 10 sec

CHANGE SET
Revision 8da1255654e844a93e387f3f80bf341bbe522870 by Bhushan Shah: 
([screenlocker] Add missing files from commit 5a0ab38613763a)
  change: add ksmserver/screenlocker/abstractlocker.h
  change: add ksmserver/screenlocker/autotests/x11lockertest.cpp
  change: add ksmserver/screenlocker/abstractlocker.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 
10 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 14/14 (100%)FILES 62/72 (86%)CLASSES 62/72 (86%)LINE 2508/4110 
(61%)CONDITIONAL 1473/2292 (64%)

By packages
  
drkonqi.parser
FILES 6/10 (60%)CLASSES 6/10 (60%)LINE 302/422 (72%)CONDITIONAL 
436/495 (88%)
drkonqi.tests.backtraceparsertest
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 77/77 (100%)CONDITIONAL 
33/50 (66%)
kioslave.desktop
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 111/149 (74%)CONDITIONAL 
40/70 (57%)
kioslave.desktop.tests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 66/66 (100%)CONDITIONAL 
26/50 (52%)
klipper
FILES 12/13 (92%)CLASSES 12/13 (92%)LINE 258/379 
(68%)CONDITIONAL 109/146 (75%)
klipper.autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 614/677 (91%)CONDITIONAL 
377/742 (51%)
ksmserver.screenlocker
FILES 10/12 (83%)CLASSES 10/12 (83%)LINE 496/1048 
(47%)CONDITIONAL 147/229 (64%)
ksmserver.screenlocker.autotests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 165/169 (98%)CONDITIONAL 
65/124 (52%)
ksmserver.screenlocker.greeter
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 99/144 (69%)CONDITIONAL 
36/60 (60%)
ksmserver.screenlocker.greeter.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 45/45 (100%)CONDITIONAL 
34/68 (50%)
libkworkspace
FILES 1/3 (33%)CLASSES 1/3 (33%)LINE 26/608 (4%)CONDITIONAL 
21/35 (60%)
runners.bookmarks
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 87/160 (54%)CONDITIONAL 
34/56 (61%)
runners.bookmarks.browsers
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 96/100 (96%)CONDITIONAL 
84/107 (79%)
runners.bookmarks.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 66/66 (100%)CONDITIONAL 
31/60 (52%)___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Jenkins-kde-ci: plasma-workspace master kf5-qt5 » Linux,gcc - Build # 231 - Fixed!

2015-10-26 Thread no-reply

GENERAL INFO

BUILD SUCCESS
Build URL: 
https://build.kde.org/job/plasma-workspace%20master%20kf5-qt5/PLATFORM=Linux,compiler=gcc/231/
Project: PLATFORM=Linux,compiler=gcc
Date of build: Mon, 26 Oct 2015 13:43:21 +
Build duration: 9 min 10 sec

CHANGE SET
Revision 8da1255654e844a93e387f3f80bf341bbe522870 by Bhushan Shah: 
([screenlocker] Add missing files from commit 5a0ab38613763a)
  change: add ksmserver/screenlocker/abstractlocker.h
  change: add ksmserver/screenlocker/autotests/x11lockertest.cpp
  change: add ksmserver/screenlocker/abstractlocker.cpp


JUNIT RESULTS

Name: (root) Failed: 0 test(s), Passed: 10 test(s), Skipped: 0 test(s), Total: 
10 test(s)

COBERTURA RESULTS

Cobertura Coverage Report
  PACKAGES 14/14 (100%)FILES 62/72 (86%)CLASSES 62/72 (86%)LINE 2508/4110 
(61%)CONDITIONAL 1473/2292 (64%)

By packages
  
drkonqi.parser
FILES 6/10 (60%)CLASSES 6/10 (60%)LINE 302/422 (72%)CONDITIONAL 
436/495 (88%)
drkonqi.tests.backtraceparsertest
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 77/77 (100%)CONDITIONAL 
33/50 (66%)
kioslave.desktop
FILES 2/3 (67%)CLASSES 2/3 (67%)LINE 111/149 (74%)CONDITIONAL 
40/70 (57%)
kioslave.desktop.tests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 66/66 (100%)CONDITIONAL 
26/50 (52%)
klipper
FILES 12/13 (92%)CLASSES 12/13 (92%)LINE 258/379 
(68%)CONDITIONAL 109/146 (75%)
klipper.autotests
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 614/677 (91%)CONDITIONAL 
377/742 (51%)
ksmserver.screenlocker
FILES 10/12 (83%)CLASSES 10/12 (83%)LINE 496/1048 
(47%)CONDITIONAL 147/229 (64%)
ksmserver.screenlocker.autotests
FILES 5/5 (100%)CLASSES 5/5 (100%)LINE 165/169 (98%)CONDITIONAL 
65/124 (52%)
ksmserver.screenlocker.greeter
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 99/144 (69%)CONDITIONAL 
36/60 (60%)
ksmserver.screenlocker.greeter.autotests
FILES 1/1 (100%)CLASSES 1/1 (100%)LINE 45/45 (100%)CONDITIONAL 
34/68 (50%)
libkworkspace
FILES 1/3 (33%)CLASSES 1/3 (33%)LINE 26/608 (4%)CONDITIONAL 
21/35 (60%)
runners.bookmarks
FILES 8/8 (100%)CLASSES 8/8 (100%)LINE 87/160 (54%)CONDITIONAL 
34/56 (61%)
runners.bookmarks.browsers
FILES 4/4 (100%)CLASSES 4/4 (100%)LINE 96/100 (96%)CONDITIONAL 
84/107 (79%)
runners.bookmarks.tests
FILES 2/2 (100%)CLASSES 2/2 (100%)LINE 66/66 (100%)CONDITIONAL 
31/60 (52%)___
Plasma-devel mailing list
Plasma-devel@kde.org
https://mail.kde.org/mailman/listinfo/plasma-devel


Review Request 125809: Fix logic (and warnings) in ModelContextMenu.qml

2015-10-26 Thread Aleix Pol Gonzalez

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

Review request for KDE Frameworks and Plasma.


Repository: plasma-framework


Description
---

It was checking that a variable existed, then it used it. Now it works like the 
rest of the properties.


Diffs
-

  src/declarativeimports/plasmacomponents/qml/ModelContextMenu.qml f8fddb7 

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


Testing
---

Booted again, tests still pass, no warnings.


Thanks,

Aleix Pol Gonzalez

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


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread David Rosca


> On Oct. 26, 2015, 10:13 a.m., Sebastian Kügler wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 81
> > 
> >
> > Why are you removing this? It seems unrelated to the rest of this patch.

Because I couldn't find any use of this and thus I don't know if it should be 
in ListView or in ScrollArea (that now has "hiddenView" id). I guess it was 
used from C++ to get handle of the ListView, but isn't used anymore?


- David


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread Marco Martin


> On Oct. 26, 2015, 9:12 a.m., Marco Martin wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 129
> > 
> >
> > this shouldn't be necessary, as hiddenView should still be just hidden 
> > when root.expandedTask != null, and when contentHeight < height shouldn't 
> > be interactive and scrollbar should be hidden
> 
> David Rosca wrote:
> You mean the whole Qt.binding is not necessary, or just the 
> !root.expandedTask test? The !root.expandedTask is there to disable 
> interactivity when only the column with icons on the left is visible and the 
> plasmoid contents is on the right in popup (eg. when you open popup with 
> clicking on Show hidden icons arrow and then select some item).

The whole Component.onCompleted: handler can be removed, yes


- Marco


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


Re: Review Request 125775: System Tray: Add ScrollArea to hidden items view

2015-10-26 Thread David Rosca


> On Oct. 26, 2015, 9:12 a.m., Marco Martin wrote:
> > applets/systemtray/package/contents/ui/ExpandedRepresentation.qml, line 129
> > 
> >
> > this shouldn't be necessary, as hiddenView should still be just hidden 
> > when root.expandedTask != null, and when contentHeight < height shouldn't 
> > be interactive and scrollbar should be hidden

You mean the whole Qt.binding is not necessary, or just the !root.expandedTask 
test? The !root.expandedTask is there to disable interactivity when only the 
column with icons on the left is visible and the plasmoid contents is on the 
right in popup (eg. when you open popup with clicking on Show hidden icons 
arrow and then select some item).


- David


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


On Oct. 24, 2015, 6:21 p.m., David Rosca wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125775/
> ---
> 
> (Updated Oct. 24, 2015, 6:21 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Bugs: 341165
> https://bugs.kde.org/show_bug.cgi?id=341165
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Show vertical scrollbar when hidden items doesn't fit in popup.
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/ExpandedRepresentation.qml 728452f 
> 
> Diff: https://git.reviewboard.kde.org/r/125775/diff/
> 
> 
> Testing
> ---
> 
> Works, although mouse scrolling works only on the scrollbar.
> 
> 
> Thanks,
> 
> David Rosca
> 
>

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


Re: Review Request 125810: Add some formfactor info to plasma-workspace applets

2015-10-26 Thread Kai Uwe Broulik

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



applets/analog-clock/metadata.desktop (line 170)


Why not "not specified = everywhere"?


- Kai Uwe Broulik


On Okt. 26, 2015, 4:26 nachm., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125810/
> ---
> 
> (Updated Okt. 26, 2015, 4:26 nachm.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> added some formfactor keywords in the applets metadata. once the explorer 
> model filters them accordingly, it should remove a bit of applets problematic 
> on the phone for one reason or another
> 
> 
> Diffs
> -
> 
>   applets/activitybar/metadata.desktop d6cf69c 
>   applets/analog-clock/metadata.desktop 4246c6e 
>   applets/batterymonitor/package/metadata.desktop a4ee702 
>   applets/calendar/metadata.desktop 5891264 
>   applets/clipboard/metadata.desktop 1d699d2 
>   applets/devicenotifier/package/metadata.desktop d71ce72 
>   applets/digital-clock/package/metadata.desktop b881453 
>   applets/lock_logout/metadata.desktop f53f541 
>   applets/mediacontroller/metadata.desktop d4ea37a 
>   applets/notifications/package/metadata.desktop 2beb20d 
>   applets/systemmonitor/cpu/metadata.desktop 76cd74d 
>   applets/systemmonitor/diskactivity/metadata.desktop 2a1ceaa 
>   applets/systemmonitor/diskusage/metadata.desktop 08344a8 
>   applets/systemmonitor/net/metadata.desktop 6671fba 
>   applets/systemtray/package/metadata.desktop 0ce0512 
> 
> Diff: https://git.reviewboard.kde.org/r/125810/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Re: Review Request 125451: RFC: Drop KScreen dependency from PlasmaShell

2015-10-26 Thread Aleix Pol Gonzalez

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

(Updated Oct. 26, 2015, 6:39 p.m.)


Review request for Plasma.


Changes
---

Adopt some changes, including the problem david spotted.


Repository: plasma-workspace


Description
---

Now that everything's in place in Qt, we can do that.

It basically removes all the translation layer between KScreen and QScreen.

NOTE: This can't be merged until Qt 5.6 is a dependency.


Diffs (updated)
-

  CMakeLists.txt 50091ea 
  shell/CMakeLists.txt e17e4a3 
  shell/desktopview.h 867cfdc 
  shell/desktopview.cpp 42a1f08 
  shell/panelview.h 9f8d4ce 
  shell/panelview.cpp 3317b52 
  shell/shellcorona.h b04b102 
  shell/shellcorona.cpp 56570b4 

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


Testing
---

Hands-on testing, it's been working quite fluid and I couldn't find a crash. 
(There could be crashes, I just didn't find them :D)


Thanks,

Aleix Pol Gonzalez

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


Re: Review Request 125810: Add some formfactor info to plasma-workspace applets

2015-10-26 Thread Marco Martin


> On Oct. 26, 2015, 5:26 p.m., Kai Uwe Broulik wrote:
> > applets/analog-clock/metadata.desktop, line 170
> > 
> >
> > Why not "not specified = everywhere"?

yeah, in some of those are probably redundant


- Marco


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


On Oct. 26, 2015, 4:26 p.m., Marco Martin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125810/
> ---
> 
> (Updated Oct. 26, 2015, 4:26 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> added some formfactor keywords in the applets metadata. once the explorer 
> model filters them accordingly, it should remove a bit of applets problematic 
> on the phone for one reason or another
> 
> 
> Diffs
> -
> 
>   applets/activitybar/metadata.desktop d6cf69c 
>   applets/analog-clock/metadata.desktop 4246c6e 
>   applets/batterymonitor/package/metadata.desktop a4ee702 
>   applets/calendar/metadata.desktop 5891264 
>   applets/clipboard/metadata.desktop 1d699d2 
>   applets/devicenotifier/package/metadata.desktop d71ce72 
>   applets/digital-clock/package/metadata.desktop b881453 
>   applets/lock_logout/metadata.desktop f53f541 
>   applets/mediacontroller/metadata.desktop d4ea37a 
>   applets/notifications/package/metadata.desktop 2beb20d 
>   applets/systemmonitor/cpu/metadata.desktop 76cd74d 
>   applets/systemmonitor/diskactivity/metadata.desktop 2a1ceaa 
>   applets/systemmonitor/diskusage/metadata.desktop 08344a8 
>   applets/systemmonitor/net/metadata.desktop 6671fba 
>   applets/systemtray/package/metadata.desktop 0ce0512 
> 
> Diff: https://git.reviewboard.kde.org/r/125810/diff/
> 
> 
> Testing
> ---
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


Review Request 125814: add kconf script to set gtk theme to breeze

2015-10-26 Thread Jonathan Riddell

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

Review request for Plasma and David Edmundson.


Repository: breeze-gtk


Description
---

sets gtk theme to breeze if it's not set or it's using oxygen/orion which we've 
set before

this is moving the code from breeze/misc and updating it for the new theme


Diffs
-

  CMakeLists.txt b977da8 
  kconf_update/CMakeLists.txt PRE-CREATION 
  kconf_update/gtkbreeze5.5.upd PRE-CREATION 
  kconf_update/main.cpp PRE-CREATION 

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


Testing
---

run the kconf_update script with no gtkrc files and with files set to orion


Thanks,

Jonathan Riddell

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


Re: Review Request 125451: RFC: Drop KScreen dependency from PlasmaShell

2015-10-26 Thread David Edmundson

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



shell/panelview.cpp (line 718)


Why is this connect in showEvent?



shell/panelview.cpp (line 729)


this won't work.

X struts are relative to the global work area (i.e all the screens) 

if you have a small laptop with a high res external screen, when you unplug 
the external screen you'll need to update the struts of a panel on the smaller 
screen.

This signal wouldn't be emitted in that case. I'm not sure what the right 
signal is.


- David Edmundson


On Oct. 26, 2015, 5:39 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125451/
> ---
> 
> (Updated Oct. 26, 2015, 5:39 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Now that everything's in place in Qt, we can do that.
> 
> It basically removes all the translation layer between KScreen and QScreen.
> 
> NOTE: This can't be merged until Qt 5.6 is a dependency.
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt 50091ea 
>   shell/CMakeLists.txt e17e4a3 
>   shell/desktopview.h 867cfdc 
>   shell/desktopview.cpp 42a1f08 
>   shell/panelview.h 9f8d4ce 
>   shell/panelview.cpp 3317b52 
>   shell/shellcorona.h b04b102 
>   shell/shellcorona.cpp 56570b4 
> 
> Diff: https://git.reviewboard.kde.org/r/125451/diff/
> 
> 
> Testing
> ---
> 
> Hands-on testing, it's been working quite fluid and I couldn't find a crash. 
> (There could be crashes, I just didn't find them :D)
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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


Re: Review Request 125814: add kconf script to set gtk theme to breeze

2015-10-26 Thread David Edmundson

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



kconf_update/main.cpp (line 54)


just return a bool?



kconf_update/main.cpp (line 65)


QStringLiteral -> QLatin1String



kconf_update/main.cpp (line 140)


Use QStandardPaths to find .config



kconf_update/main.cpp (line 171)


not needed


- David Edmundson


On Oct. 26, 2015, 5:40 p.m., Jonathan Riddell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125814/
> ---
> 
> (Updated Oct. 26, 2015, 5:40 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: breeze-gtk
> 
> 
> Description
> ---
> 
> sets gtk theme to breeze if it's not set or it's using oxygen/orion which 
> we've set before
> 
> this is moving the code from breeze/misc and updating it for the new theme
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt b977da8 
>   kconf_update/CMakeLists.txt PRE-CREATION 
>   kconf_update/gtkbreeze5.5.upd PRE-CREATION 
>   kconf_update/main.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125814/diff/
> 
> 
> Testing
> ---
> 
> run the kconf_update script with no gtkrc files and with files set to orion
> 
> 
> Thanks,
> 
> Jonathan Riddell
> 
>

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


Review Request 125810: Add some formfactor info to plasma-workspace applets

2015-10-26 Thread Marco Martin

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

Review request for Plasma.


Repository: plasma-workspace


Description
---

added some formfactor keywords in the applets metadata. once the explorer model 
filters them accordingly, it should remove a bit of applets problematic on the 
phone for one reason or another


Diffs
-

  applets/activitybar/metadata.desktop d6cf69c 
  applets/analog-clock/metadata.desktop 4246c6e 
  applets/batterymonitor/package/metadata.desktop a4ee702 
  applets/calendar/metadata.desktop 5891264 
  applets/clipboard/metadata.desktop 1d699d2 
  applets/devicenotifier/package/metadata.desktop d71ce72 
  applets/digital-clock/package/metadata.desktop b881453 
  applets/lock_logout/metadata.desktop f53f541 
  applets/mediacontroller/metadata.desktop d4ea37a 
  applets/notifications/package/metadata.desktop 2beb20d 
  applets/systemmonitor/cpu/metadata.desktop 76cd74d 
  applets/systemmonitor/diskactivity/metadata.desktop 2a1ceaa 
  applets/systemmonitor/diskusage/metadata.desktop 08344a8 
  applets/systemmonitor/net/metadata.desktop 6671fba 
  applets/systemtray/package/metadata.desktop 0ce0512 

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


Testing
---


Thanks,

Marco Martin

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


Review Request 125812: Fix icon-based status notifier icons

2015-10-26 Thread Aleix Pol Gonzalez

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

Review request for Plasma.


Repository: plasma-workspace


Description
---

Normally check if the value is correct by checking if it's different to null.
Use a property rather than a function, so that if a value changes, it gets 
picked up. Additionally this will make it share the value rather than rendering 
the icon twice (one for each use).


Diffs
-

  applets/systemtray/package/contents/ui/StatusNotifierItem.qml d2cc2f6 

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


Testing
---

I don't get empty spaces anymore.


Thanks,

Aleix Pol Gonzalez

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


Re: Review Request 125812: Fix icon-based status notifier icons

2015-10-26 Thread David Edmundson

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

Ship it!


Ship It!

- David Edmundson


On Oct. 26, 2015, 5:03 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125812/
> ---
> 
> (Updated Oct. 26, 2015, 5:03 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> ---
> 
> Normally check if the value is correct by checking if it's different to null.
> Use a property rather than a function, so that if a value changes, it gets 
> picked up. Additionally this will make it share the value rather than 
> rendering the icon twice (one for each use).
> 
> 
> Diffs
> -
> 
>   applets/systemtray/package/contents/ui/StatusNotifierItem.qml d2cc2f6 
> 
> Diff: https://git.reviewboard.kde.org/r/125812/diff/
> 
> 
> Testing
> ---
> 
> I don't get empty spaces anymore.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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


Re: Review Request 125809: Fix logic (and warnings) in ModelContextMenu.qml

2015-10-26 Thread Aleix Pol Gonzalez

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

(Updated Oct. 26, 2015, 5:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for KDE Frameworks and Plasma.


Changes
---

Submitted with commit 36288df5ac48901134ca98ff8131d87ad434e2a1 by Aleix Pol to 
branch master.


Repository: plasma-framework


Description
---

It was checking that a variable existed, then it used it. Now it works like the 
rest of the properties.


Diffs
-

  src/declarativeimports/plasmacomponents/qml/ModelContextMenu.qml f8fddb7 

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


Testing
---

Booted again, tests still pass, no warnings.


Thanks,

Aleix Pol Gonzalez

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


Review Request 125811: detect gtk engine

2015-10-26 Thread Jonathan Riddell

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

Review request for Plasma and David Edmundson.


Repository: breeze-gtk


Description
---

first attempt at detecting gtk 2 pixmap engine as runtime dependency

but it doesn't work if you set cmake path to different from gtk engine

and I don't know if it works on distros that use /usr/lib64/ rather than 
/usr/lib/x86_86-linux-gnu


Diffs
-

  CMakeLists.txt b977da8 
  cmake/FindGTKEngine.cmake PRE-CREATION 
  cmake/FindGTKEngine.cmake~ PRE-CREATION 
  cmake/GTKEngineConfig.cmake PRE-CREATION 

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


Testing
---

works for me unless I install outwith /usr/


Thanks,

Jonathan Riddell

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


Re: Review Request 125811: detect gtk engine

2015-10-26 Thread David Edmundson

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



CMakeLists.txt (line 14)


remove



cmake/FindGTKEngine.cmake (line 9)


what if the GTK version isn't 2.10?



cmake/FindGTKEngine.cmake~ (line 1)


git rm this file



cmake/GTKEngineConfig.cmake (line 3)


that's not distro agnostic

I think there's a cmake var for libpath


- David Edmundson


On Oct. 26, 2015, 4:43 p.m., Jonathan Riddell wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125811/
> ---
> 
> (Updated Oct. 26, 2015, 4:43 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: breeze-gtk
> 
> 
> Description
> ---
> 
> first attempt at detecting gtk 2 pixmap engine as runtime dependency
> 
> but it doesn't work if you set cmake path to different from gtk engine
> 
> and I don't know if it works on distros that use /usr/lib64/ rather than 
> /usr/lib/x86_86-linux-gnu
> 
> 
> Diffs
> -
> 
>   CMakeLists.txt b977da8 
>   cmake/FindGTKEngine.cmake PRE-CREATION 
>   cmake/FindGTKEngine.cmake~ PRE-CREATION 
>   cmake/GTKEngineConfig.cmake PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/125811/diff/
> 
> 
> Testing
> ---
> 
> works for me unless I install outwith /usr/
> 
> 
> Thanks,
> 
> Jonathan Riddell
> 
>

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


Re: Review Request 125809: Fix logic (and warnings) in ModelContextMenu.qml

2015-10-26 Thread David Edmundson

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

Ship it!


Ship It!

- David Edmundson


On Oct. 26, 2015, 4:07 p.m., Aleix Pol Gonzalez wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125809/
> ---
> 
> (Updated Oct. 26, 2015, 4:07 p.m.)
> 
> 
> Review request for KDE Frameworks and Plasma.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> ---
> 
> It was checking that a variable existed, then it used it. Now it works like 
> the rest of the properties.
> 
> 
> Diffs
> -
> 
>   src/declarativeimports/plasmacomponents/qml/ModelContextMenu.qml f8fddb7 
> 
> Diff: https://git.reviewboard.kde.org/r/125809/diff/
> 
> 
> Testing
> ---
> 
> Booted again, tests still pass, no warnings.
> 
> 
> Thanks,
> 
> Aleix Pol Gonzalez
> 
>

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


Re: Review Request 125812: Fix icon-based status notifier icons

2015-10-26 Thread Aleix Pol Gonzalez

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

(Updated Oct. 26, 2015, 5:22 p.m.)


Status
--

This change has been marked as submitted.


Review request for Plasma.


Changes
---

Submitted with commit 867132ffc6799c19bc27ff48a14106f43ac4539d by Aleix Pol to 
branch master.


Repository: plasma-workspace


Description
---

Normally check if the value is correct by checking if it's different to null.
Use a property rather than a function, so that if a value changes, it gets 
picked up. Additionally this will make it share the value rather than rendering 
the icon twice (one for each use).


Diffs
-

  applets/systemtray/package/contents/ui/StatusNotifierItem.qml d2cc2f6 

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


Testing
---

I don't get empty spaces anymore.


Thanks,

Aleix Pol Gonzalez

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