[kde-edu]: Review Request: Audio for kwordquiz.

[Peter Hedlund]

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


Thank you for continuing to work on this. Unfortunately the patch does not apply cleanly for me. Are you sure you use a fully updated source? I hope to be able to test the next version.


trunk/KDE/kdeedu/kwordquiz/src/kwordquizui.rc
<http://reviewboard.kde.org/r/2860/#comment3652>

    Delete



trunk/KDE/kdeedu/kwordquiz/src/kwqquizmodel.h
<http://reviewboard.kde.org/r/2860/#comment3653>

    I don't understand your approach with hidden columns. To me it seems it must be better to add an additional custom role in kwqtablemodel.h. I foresee the addition of at least image and multiple choice roles in the future.
    
      enum roles {
        KeyboardLayoutRole = Qt::UserRole,
        AudioRole
      };



trunk/KDE/kdeedu/kwordquiz/src/kwqquizmodel.h
<http://reviewboard.kde.org/r/2860/#comment3654>

    You don't use this. Delete?



trunk/KDE/kdeedu/kwordquiz/src/kwqquizmodel.cpp
<http://reviewboard.kde.org/r/2860/#comment3657>

    Is this needed? I would prefer Qt and KDE includes only.



trunk/KDE/kdeedu/kwordquiz/src/kwqquizmodel.cpp
<http://reviewboard.kde.org/r/2860/#comment3658>

    Use a custom role to retrieve the url instead.



trunk/KDE/kdeedu/kwordquiz/src/kwqtablemodel.h
<http://reviewboard.kde.org/r/2860/#comment3659>

    Use a custom role instead.



trunk/KDE/kdeedu/kwordquiz/src/qaview.cpp
<http://reviewboard.kde.org/r/2860/#comment3660>

    Aren't we using 0 instead of NULL in KDE?


- Peter


On 2010-02-08 14:49:33, Stanislaw Gruszka wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://reviewboard.kde.org/r/2860/
> -----------------------------------------------------------
> 
> (Updated 2010-02-08 14:49:33)
> 
> 
> Review request for KDE-Edu.
> 
> 
> Summary
> -------
> 
> Basic functionality  in (for qaview). Still some work to do: configuration, change Ctrl+P (Ctrl+U not work ?), downloading files, maybe more.
> 
> 
> Diffs
> -----
> 
>   trunk/KDE/kdeedu/kwordquiz/src/flashview.cpp 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwordquiz.h 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwordquiz.cpp 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwordquizui.rc 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwqquizmodel.h 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwqquizmodel.cpp 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwqtablemodel.h 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwqtablemodel.cpp 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/kwqtableview.cpp 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/multipleview.cpp 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/qaview.h 1079126 
>   trunk/KDE/kdeedu/kwordquiz/src/qaview.cpp 1079126 
> 
> Diff: http://reviewboard.kde.org/r/2860/diff
> 
> 
> Testing
> -------
> 
> Patch works. I note some strange system tray blinking, when playing audio. Looks phonon add icon to tray and remove it. Not investigate more.
> 
> 
> Thanks,
> 
> Stanislaw
> 
>