Review Request 111263: Revamping the filesystem browser in the plasma-mediacenter

[Shantanu Tushar]

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


looks good, just fix these issues


browsingbackends/localfiles/localfilesabstractbackend.cpp
<http://git.reviewboard.kde.org/r/111263/#comment25757>

    Remove this before pushing



browsingbackends/localfiles/localfilesabstractbackend.cpp
<http://git.reviewboard.kde.org/r/111263/#comment25758>

    KDE/KUrl, for consistency



browsingbackends/localfiles/localfilesabstractbackend.cpp
<http://git.reviewboard.kde.org/r/111263/#comment25755>

    variable names should tell you what they represent. In this case, m_expand = true means placesModel is not being shown.
    Ideally, you should reverse the logic and call the variable m_isShowingPlacesModel. (So all your true's will become false's and vice-versa)



browsingbackends/localfiles/localfilesabstractbackend.cpp
<http://git.reviewboard.kde.org/r/111263/#comment25756>

    Remove the qDebug() before pushing



browsingbackends/localfiles/localfilesabstractbackend.cpp
<http://git.reviewboard.kde.org/r/111263/#comment25754>

    m_placesModel, plural



browsingbackends/localfiles/localfilesabstractmodel.cpp
<http://git.reviewboard.kde.org/r/111263/#comment25759>

    combine these into a single return statement



browsingbackends/localfiles/localplacesmodel.cpp
<http://git.reviewboard.kde.org/r/111263/#comment25760>

    placesModel, plural


- Shantanu Tushar


On June 26, 2013, 7:22 p.m., Akshay Ratan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/111263/
> -----------------------------------------------------------
> 
> (Updated June 26, 2013, 7:22 p.m.)
> 
> 
> Review request for Plasma, Fabian Riethmayer, Shantanu Tushar, Sinny Kumari, and Sujith Haridasan.
> 
> 
> Description
> -------
> 
> The File explorer/browser in the plasma-mediacenter has been revamped. The side-panel has been removed. Now the drives and other system directories appears on clicking "Browse Pics/Music/Videos" instead of "home" folder contents appeaaring earlier after selecting this particular option. 
> 
> 
> Diffs
> -----
> 
>   browsingbackends/localfiles/localfilesabstractbackend.h b7cc909 
>   browsingbackends/localfiles/localfilesabstractbackend.cpp 5768fb3 
>   browsingbackends/localfiles/localfilesabstractmodel.h c4c8be3 
>   browsingbackends/localfiles/localfilesabstractmodel.cpp 6c66b43 
>   browsingbackends/localfiles/localmusic/CMakeLists.txt 4b804ae 
>   browsingbackends/localfiles/localpictures/CMakeLists.txt c7ba5fa 
>   browsingbackends/localfiles/localplacesmodel.h PRE-CREATION 
>   browsingbackends/localfiles/localplacesmodel.cpp PRE-CREATION 
>   browsingbackends/localfiles/localvideos/CMakeLists.txt 15677d7 
> 
> Diff: http://git.reviewboard.kde.org/r/111263/diff/
> 
> 
> Testing
> -------
> 
> Major things working fine , but there might be certain issues regarding the correct folder contents shown inside any folder in any particular directory.
> 
> Further malformed url might appear on clicking certain drives. However this can be fixed later :)
> 
> 
> Thanks,
> 
> Akshay Ratan
> 
>

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