Review Request 114378: Add a hideOnWindowDeactivate property to PlasmaCore.Dialog

Sebastian Kügler sebas at kde.org
Tue Dec 10 15:26:28 UTC 2013



> On Dec. 10, 2013, 2:40 p.m., Sebastian Kügler wrote:
> > src/declarativeimports/core/dialog.h, line 93
> > <http://git.reviewboard.kde.org/r/114378/diff/1/?file=223641#file223641line93>
> >
> >     READ hideOnWindowDeactivate?
> >     
> >     Or is there a reason to use is*** here, such as an API collision?
> 
> Martin Gräßlin wrote:
>     normal property naming scheme. if it is a bool it start's with "is". I don't mind if you want to have it different, but it's at least consistant with the other boolean properties.

I think we (and Qt) moved away from the is* scheme, unless there's a collision. The other occurrence in that file is I think a left-over. The vast majority in Plasma and KDE code doesn't use is* anymore. I'd prefer it without the is, but it's not a big deal, and this detail gets hidden by the property name to the user anyway. Change or commit-as-is according to whether I could convince you or not. :)


- Sebastian


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/114378/#review45460
-----------------------------------------------------------


On Dec. 10, 2013, 2:14 p.m., Martin Gräßlin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114378/
> -----------------------------------------------------------
> 
> (Updated Dec. 10, 2013, 2:14 p.m.)
> 
> 
> Review request for Plasma and Sebastian Kügler.
> 
> 
> Repository: plasma-framework
> 
> 
> Description
> -------
> 
> This property is meant to bring back the functionality provided by PopupApplet. If the property is set to true the dialog gets hidden when it loses focus.
> 
> 
> Diffs
> -----
> 
>   src/declarativeimports/core/dialog.h d612882 
>   src/declarativeimports/core/dialog.cpp 04ef7e4 
> 
> Diff: http://git.reviewboard.kde.org/r/114378/diff/
> 
> 
> Testing
> -------
> 
> Tested with CompactApplet in the desktop qmlpackages. It works, but it shouldn't be set to true there. It needs to be set only on the windows it should have. To test I recommend to switch to focus follows mouse. Whenever the window closes in an unexpected way, it's wrong.
> 
> 
> Thanks,
> 
> Martin Gräßlin
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20131210/b7013d16/attachment-0001.html>


More information about the Plasma-devel mailing list