Review Request 119618: add theme selection for screen lockers

Marco Martin notmart at gmail.com
Wed Aug 6 09:20:53 UTC 2014



> On Aug. 5, 2014, 3:35 p.m., David Edmundson wrote:
> > ksmserver/screenlocker/kcm/package/contents/ui/main.qml, line 128
> > <https://git.reviewboard.kde.org/r/119618/diff/1/?file=296885#file296885line128>
> >
> >     what's this for? I assume you're working round something.
> >     
> >     (and don't reply here, reply in code comments :) )
> 
> Marco Martin wrote:
>     i'll add it in the comments as well ;)
>     basically, list.setCurrentIndex doesn't work while the model is getting loaded, so list.currentIndex = index in a component.onCompleted of the delegate, doesn't work, restarting a timer when a delegate gets created, seems the only place where we can approximate "set the property when the view really is done loading"
> 
> David Edmundson wrote:
>     One approach I've seen done in the past (not sure if it'll work here) is to have a currentPlugin string property on the root view then in the delegate do
>     
>     Component.onCompleted: {
>       if (root.currentPluginString == myString) {
>         view.currentIndex = index
>       }
>     }
> 
> Marco Martin wrote:
>     yeah, that was the first thing i tried,
>     but if the current item is not the last, and/or is not the last to be created, view.currentIndex = index will have no effect (yep, pretty weird i know)
> 
> David Edmundson wrote:
>     Confirmed. It's deliberate in Qt.
>     
>     QQuickItemView::setCurrentIndex(int index)
>       if (d->inRequest)  // currently creating item
>             return;
>     
>     
>     Mini unit test if people want it: http://paste.kde.org/patrq6fmm
> 
> David Edmundson wrote:
>     Original timer seems to be a sensible workaround, I can't find anything better :(
>     
>     We may as well set the time to 0. We only need to treat it like a queued connection. Putting it to a real value makes it look like a far bigger hack than it really is, plus it makes it slower for no reason
>     
>     I'm starting to think we should have a utils singleton in kcoreaddons somewhere with
>     
>      Q_INVOKABLE void invokeQueuedMethod(QObject *target, QString slot) {
>         QMetaObject::invokeMethod(target, slot, Qt::QueuedConnection)
>      }
>     
>     it'd get rid of a lot timers and the code would be a bit more readable too. This isn't the first time and won't be the last time we'll want to do this. It's not very declarative, but neither is using timers at all.
>     
>     Thoughts?
> 
> Marco Martin wrote:
>     could be nice..
>     maybe maybe there could be a way to pass parameters as well? maybe if the parameter is a qjsvalue it can be called with the syntax of a normal callback
>     kcoreAddons.invokeQueued(function("arg") { do stuff})
>     implementation would be a bit dirtier tough
> 
> David Edmundson wrote:
>     We can do args on the above version; I avoided suggesting your version because I would have no idea how to do it :D
>     If you could manage it, that'd be super sexy.

i *think* i know how would be done.. (not sure yet how make it async tough) only thing, the implementation would be quite ugly


- Marco


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


On Aug. 5, 2014, 4:14 p.m., Marco Martin wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/119618/
> -----------------------------------------------------------
> 
> (Updated Aug. 5, 2014, 4:14 p.m.)
> 
> 
> Review request for Plasma.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> one thing that was requested a lot as well, is an override on the screen locker package as is done in the splash screen as well (master already supports reading it from config, the kcm to set that was missing tough)
> 
> 
> Diffs
> -----
> 
>   ksmserver/screenlocker/kcm/CMakeLists.txt 17e4f70 
>   ksmserver/screenlocker/kcm/kcm.h PRE-CREATION 
>   ksmserver/screenlocker/kcm/kcm.cpp d74ab75 
>   ksmserver/screenlocker/kcm/kcm.ui 1cc2653 
>   ksmserver/screenlocker/kcm/package/contents/ui/main.qml PRE-CREATION 
>   ksmserver/screenlocker/kcm/package/metadata.desktop PRE-CREATION 
>   lookandfeel/contents/lockscreen/screenshot.png PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/119618/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> locker.png
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/08/05/1174ed52-d86c-4345-9790-0cb5dd8f75ca__locker.png
> 
> 
> Thanks,
> 
> Marco Martin
> 
>

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


More information about the Plasma-devel mailing list