Review Request 113346: BugFix: 317902 - Issue a warning when transcoding is not possible

[Matěj Laitl]

> On Oct. 19, 2013, 7:49 p.m., Matěj Laitl wrote:
> > src/core-impl/collections/support/CollectionLocationDelegateImpl.h, lines 41-42
> > <http://git.reviewboard.kde.org/r/113346/diff/3/?file=203475#file203475line41>
> >
> >     TranscodingAssistantDialog knows what encoders are available (see its populateFormatList() method), no need to pass this as an argument. Also, please mind Amarok coding style (space between "transcode(" and "const")
> 
> Jai  Luthra wrote:
>     Inside populateFormatList(), if available.isEmpty(), I think I should call (and define) another method TranscodingAssistantDialog::encoderNotFound() rather than changing the UI from the if block itself. Right?

That's up to you, the code to show the warning is simple, no problem with embedding it into populateFormatList().

Another idea: add "You may check <i>%1</i> in order to skip this dialog for future transfers" - where %1 is text of the checkbox button to do this. (please do it this way to ensure that the text is exactly the same even in translations)


- Matěj


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


On Oct. 19, 2013, 11:12 a.m., Jai  Luthra wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/113346/
> -----------------------------------------------------------
> 
> (Updated Oct. 19, 2013, 11:12 a.m.)
> 
> 
> Review request for Amarok.
> 
> 
> Bugs: 317902
>     https://bugs.kde.org/show_bug.cgi?id=317902
> 
> 
> Repository: amarok
> 
> 
> Description
> -------
> 
> When ffmpeg is not available, the transcoding dialog will not be skipped; rather it will add a note for the user to install an encoder.
> 
> 
> Diffs
> -----
> 
>   src/core-impl/collections/support/CollectionLocationDelegateImpl.h 9d90580 
>   src/core-impl/collections/support/CollectionLocationDelegateImpl.cpp 14fd251 
>   src/core/collections/CollectionLocation.cpp 2385570 
>   src/core/collections/CollectionLocationDelegate.h 3c80d07 
>   src/transcoding/TranscodingAssistantDialog.h 0acb701 
>   src/transcoding/TranscodingAssistantDialog.cpp 17fc250 
>   src/transcoding/TranscodingAssistantDialog.ui 0ad83b2 
> 
> Diff: http://git.reviewboard.kde.org/r/113346/diff/
> 
> 
> Testing
> -------
> 
> * Compiles
> * Works as expected
> 
> 
> Thanks,
> 
> Jai  Luthra
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/amarok-devel/attachments/20131019/1315e8c7/attachment.html>