[Marble-devel] Review Request 114814: Distinguish between expanded and unexpanded icon state in Marble's Files View

[Dennis Nienhüser]

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


Looks good :-)


src/lib/marble/TreeViewDecoratorModel.h
<https://git.reviewboard.kde.org/r/114814/#comment33285>

    Ideally use passive names for signals and active names for slots, so better sth like
    trackExpandedState() and trackCollapsedState()



src/lib/marble/TreeViewDecoratorModel.cpp
<https://git.reviewboard.kde.org/r/114814/#comment33284>

    I'd simplify that to (replacing the foreach loop)
    bool const expandedState = m_expandedRows.contains( QPersistentModelIndex( proxyIndex ) );
    



src/lib/marble/TreeViewDecoratorModel.cpp
<https://git.reviewboard.kde.org/r/114814/#comment33282>

    This check is redundant to the foreach loop below (which will not be entered when size = 0), so it's not needed.



src/lib/marble/TreeViewDecoratorModel.cpp
<https://git.reviewboard.kde.org/r/114814/#comment33286>

    Shall we remove the liststyle icon handling in GeoDataTreeModel then? Seems we can handle all here.
    
    



src/lib/marble/TreeViewDecoratorModel.cpp
<https://git.reviewboard.kde.org/r/114814/#comment33283>

    I'd use a QList for m_expandedRows and then call m_expandedRows.removeAll( pindex ); instead of the for loop.


- Dennis Nienhüser


On Jan. 2, 2014, 5:34 p.m., Levente Kurusa wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/114814/
> -----------------------------------------------------------
> 
> (Updated Jan. 2, 2014, 5:34 p.m.)
> 
> 
> Review request for Marble, Dennis Nienhüser and Torsten Rahn.
> 
> 
> Repository: marble
> 
> 
> Description
> -------
> 
> GCI task: http://www.google-melange.com/gci/task/view/google/gci2013/5903596897959936
> 
> 
> Diffs
> -----
> 
>   src/lib/marble/FileViewWidget.cpp 59d8343 
>   src/lib/marble/TreeViewDecoratorModel.h 45fb5d8 
>   src/lib/marble/TreeViewDecoratorModel.cpp 4d78708 
> 
> Diff: https://git.reviewboard.kde.org/r/114814/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> KML file used for testing
>   https://git.reviewboard.kde.org/media/uploaded/files/2014/01/02/b8170760-8a34-4cc7-820e-cb75b28ed1d7__list.kml
> 
> 
> Thanks,
> 
> Levente Kurusa
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/marble-devel/attachments/20140102/2962eea6/attachment.html>