Review Request: Allow hiding of items in the Kate Sessions Plasmoid

[Aaron Seigo]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://svn.reviewboard.kde.org/r/4264/#review7430
-----------------------------------------------------------

Ship it!


i've comment on some minor improvements to the code and UI that should be made, imho, but the patch looks generally ready. if you can tidy up these last few items, feel free to commit this improvement! thanks for the patch... :)


/trunk/KDE/kdesdk/kate/plasma/session/katesessionConfig.ui
<http://svn.reviewboard.kde.org/r/4264/#comment7633>

    this label is probably unnecessary: checking items is well understood to mean "select", and with the title just being "Sessions to show" what that selection means will be clear.



/trunk/KDE/kdesdk/kate/plasma/session/katesessionapplet.h
<http://svn.reviewboard.kde.org/r/4264/#comment7636>

    following naming conventions, this should be:
    
    QStringList hideList() const;



/trunk/KDE/kdesdk/kate/plasma/session/katesessionapplet.cpp
<http://svn.reviewboard.kde.org/r/4264/#comment7634>

    i wouldn't bother with m_hideList in the KateSessionApplet class. the only time it is used, it is read from the configuration file first!
    
    instead, here i would just put:
    
    const QStringList hideList = config().readEntry("hideList", QStringList());



/trunk/KDE/kdesdk/kate/plasma/session/katesessionapplet.cpp
<http://svn.reviewboard.kde.org/r/4264/#comment7632>

    perhaps just "Sessions to show"
    



/trunk/KDE/kdesdk/kate/plasma/session/katesessionapplet.cpp
<http://svn.reviewboard.kde.org/r/4264/#comment7635>

    without m_hideList this just becomes:
    
    config().writeEntry("hideList", m_config->hideList());


- Aaron


On 2010-09-06 06:31:35, Yuen Hoe Lim wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://svn.reviewboard.kde.org/r/4264/
> -----------------------------------------------------------
> 
> (Updated 2010-09-06 06:31:35)
> 
> 
> Review request for Kate and Plasma.
> 
> 
> Summary
> -------
> 
> Allows the user to specify which items to show or hide in the Kate Sessions Plasmoid using a configuration interface. Two possible use-cases for this:
> 
> 1) Allow users who just want quick access to their projects to remove the default three items (Start Kate, New Session etc), which can take up a lot of space since the applet is usually quite small.
> 
> 2) Allow multiple applets displaying different lists, for example having applets in different activities each displaying only sessions related to that activity.
> 
> The Kate Sessions Plasmoid currently does not have a configuration interface so I thought this is probably no harm :) I have attached a screenshot of the configuration UI. I'm somewhat inexperienced with QtDesigner so feedback on how I could improve the patch code or the config UI are most welcome :)
> 
> 
> Diffs
> -----
> 
>   /trunk/KDE/kdesdk/kate/plasma/session/CMakeLists.txt 1171815 
>   /trunk/KDE/kdesdk/kate/plasma/session/katesessionConfig.ui PRE-CREATION 
>   /trunk/KDE/kdesdk/kate/plasma/session/katesessionapplet.h 1171815 
>   /trunk/KDE/kdesdk/kate/plasma/session/katesessionapplet.cpp 1171815 
> 
> Diff: http://svn.reviewboard.kde.org/r/4264/diff
> 
> 
> Testing
> -------
> 
> Tested on trunk on my computer. Works as far as I can tell.
> 
> 
> Screenshots
> -----------
> 
> Config UI Screenshot
>   http://svn.reviewboard.kde.org/r/4264/s/428/
> Updated config UI screenshot
>   http://svn.reviewboard.kde.org/r/4264/s/497/
> 
> 
> Thanks,
> 
> Yuen Hoe
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.kde.org/pipermail/plasma-devel/attachments/20100906/689e50a2/attachment-0001.htm