Re: Review Request 129673: [recentlyPLayedList] Add keyboard shortcuts

Harald Sitter <[email protected]>
Newsgroups gmane.comp.kde.users.multimedia
Message-ID <[email protected]>

> On Dec. 20, 2016, 9:55 a.m., Harald Sitter wrote:
> > src/app/recentlyPlayedList.cpp, line 46
> > <https://git.reviewboard.kde.org/r/129673/diff/1/?file=487860#file487860line46>
> >
> >     Why is this change needed?
> >     
> >     Also, for future reference. Please init one object at a time
> >     
> >     ```
> >     clear = new
> >     clear.setText
> >     ...
> >     connect(clear, ...)
> >     
> >     remove = new
> >     remove.setText
> >     ...
> >     connect(clear, ...)
> >     ```
> >     
> >     It's easier to read that way.
> 
> Anthony Fieroni wrote:
>     It's needed when you want to use only shortcut without menu.

Right, please split this into two reviews. One to fix the removal bug and one to enable shortcut support.


- Harald


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


On Dec. 20, 2016, 5:06 a.m., Anthony Fieroni wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/129673/
> -----------------------------------------------------------
> 
> (Updated Dec. 20, 2016, 5:06 a.m.)
> 
> 
> Review request for KDE Multimedia and Harald Sitter.
> 
> 
> Repository: dragon
> 
> 
> Description
> -------
> 
> + Correct unspected behavior QString operator== not work for toDisplayString() e.g. file:$HOME/???????/FPS_test_1080p23.976_L4.1.mkv
> 
> 
> Diffs
> -----
> 
>   src/app/recentlyPlayedList.cpp 2c25e7f 
>   src/app/stateChange.cpp 7bf4038 
> 
> Diff: https://git.reviewboard.kde.org/r/129673/diff/
> 
> 
> Testing
> -------
> 
> 
> File Attachments
> ----------------
> 
> Actions
>   https://git.reviewboard.kde.org/media/uploaded/files/2016/12/20/fad5b9ab-9ffb-4220-a0f4-5a25845c6507__Screenshot_20161220_070541.png
> 
> 
> Thanks,
> 
> Anthony Fieroni
> 
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.