Review Request 114223: Moving popupmenu to mediacenter and making it more generic

Shantanu Tushar shantanu at kde.org
Sat Nov 30 14:42:07 UTC 2013


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


Does show the menu on right-click or click and hold. However the following needs to be fixed-
1. Cancel button does nothing
2. After clicking "add to playlist", the screen becomes blank. It should go back to the browser.



shells/newshell/package/contents/ui/mediacenter.qml
<http://git.reviewboard.kde.org/r/114223/#comment32076>

    This is not needed. The signal is already there in MediaBrowser.qml



shells/newshell/package/contents/ui/mediacenter.qml
<http://git.reviewboard.kde.org/r/114223/#comment32077>

    This if condition is not needed. You are alrady checking this in getPopupMenu() (L354)



shells/newshell/package/contents/ui/mediacenter.qml
<http://git.reviewboard.kde.org/r/114223/#comment32078>

    by using var you are declaring a new local variable, hence you will not be using the property you defined on L35. Just say-
    popupMenuInstance = getPopupMenu



shells/newshell/package/contents/ui/mediacenter.qml
<http://git.reviewboard.kde.org/r/114223/#comment32079>

    If this code is not needed, please remove it



shells/newshell/package/contents/ui/mediacenter.qml
<http://git.reviewboard.kde.org/r/114223/#comment32080>

    these property declarations can be moved to PopupMenu.qml



shells/newshell/package/contents/ui/mediacenter.qml
<http://git.reviewboard.kde.org/r/114223/#comment32081>

    This is why choosing the Cancel option does nothing. You should call root.goBack() here.


- Shantanu Tushar


On Nov. 30, 2013, 1:54 p.m., Sujith Haridasan wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/114223/
> -----------------------------------------------------------
> 
> (Updated Nov. 30, 2013, 1:54 p.m.)
> 
> 
> Review request for Plasma, Shantanu Tushar and Sinny Kumari.
> 
> 
> Repository: plasma-mediacenter
> 
> 
> Description
> -------
> 
> Moving popup menu to mediacenter.qml and hence making it more generic. popup menu is pushed to the pagestack and hence it helps the user in keyboard navigation using Esc key too.
> 
> 
> Diffs
> -----
> 
>   mediaelements/mediabrowser/MediaBrowser.qml ec76b99 
>   shells/newshell/package/contents/ui/mediacenter.qml 860b794 
> 
> Diff: http://git.reviewboard.kde.org/r/114223/diff/
> 
> 
> Testing
> -------
> 
> Tested by moving the popup menu to mediacenter by launching pmc. And also when pressed the Esc key it takes user to the files section of the media browser.
> 
> 
> Thanks,
> 
> Sujith Haridasan
> 
>

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


More information about the Plasma-devel mailing list