Review Request 124980: Add org.kde.plasma.private.sessions with a SessionModel

Martin Gräßlin mgraesslin at kde.org
Mon Aug 31 06:25:51 UTC 2015


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



components/sessionsprivate/sessionsmodel.h (line 56)
<https://git.reviewboard.kde.org/r/124980/#comment58553>

    suggestion: enum class, and then drop the Role from each name. Usage will be e.g. Roles::Icon



components/sessionsprivate/sessionsmodel.h (lines 73 - 75)
<https://git.reviewboard.kde.org/r/124980/#comment58556>

    add override



components/sessionsprivate/sessionsmodel.h (line 83)
<https://git.reviewboard.kde.org/r/124980/#comment58551>

    Is KDisplayManager using (blocking) DBus calls? If yes, I think we should design this to get it async.



components/sessionsprivate/sessionsmodel.h (line 85)
<https://git.reviewboard.kde.org/r/124980/#comment58552>

    Playing Milian: cetero censeo QList delendam esset.
    
    -> use QVector ;-)



components/sessionsprivate/sessionsmodel.cpp (lines 49 - 50)
<https://git.reviewboard.kde.org/r/124980/#comment58554>

    depending on how often it gets read I suggest to cache the value



components/sessionsprivate/sessionsmodel.cpp (line 73)
<https://git.reviewboard.kde.org/r/124980/#comment58558>

    The way to go is to emit the dbus call and connect to the signal for locked.
    
    so
    1. Connect globally to ksld's locked signal
    2. Track whether screen is locked initially
    3. if locked -> switch directly
    4. if not locked -> lock, wait for signal
    5. on signal: switch



components/sessionsprivate/sessionsmodel.cpp (line 80)
<https://git.reviewboard.kde.org/r/124980/#comment58555>

    either qCDebug or remove ;-)



components/sessionsprivate/sessionsmodel.cpp (line 111)
<https://git.reviewboard.kde.org/r/124980/#comment58557>

    same comment as for other debug statement


- Martin Gräßlin


On Aug. 29, 2015, 4:14 p.m., Kai Uwe Broulik wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124980/
> -----------------------------------------------------------
> 
> (Updated Aug. 29, 2015, 4:14 p.m.)
> 
> 
> Review request for Plasma and Martin Gräßlin.
> 
> 
> Repository: plasma-workspace
> 
> 
> Description
> -------
> 
> This way it can be used in the lock screen, user switcher, and a user switcher plasmoid I'm planning.
> 
> Compared to the one in the lock screen it can also get the user's full name and avatar, and some other information.
> 
> Perhaps we could also try to have it wait for the screen to actually lock before switching sessions?
> 
> 
> Diffs
> -----
> 
>   components/CMakeLists.txt 21fc61c 
>   components/sessionsprivate/CMakeLists.txt PRE-CREATION 
>   components/sessionsprivate/qmldir PRE-CREATION 
>   components/sessionsprivate/sessionsmodel.h PRE-CREATION 
>   components/sessionsprivate/sessionsmodel.cpp PRE-CREATION 
>   components/sessionsprivate/sessionsprivateplugin.h PRE-CREATION 
>   components/sessionsprivate/sessionsprivateplugin.cpp PRE-CREATION 
> 
> Diff: https://git.reviewboard.kde.org/r/124980/diff/
> 
> 
> Testing
> -------
> 
> Lists my sessions, allows to switch between them and start new ones. Locking when creating a new session is missing.
> 
> 
> Thanks,
> 
> Kai Uwe Broulik
> 
>

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


More information about the Plasma-devel mailing list