D19782: Allow dolphin to auto-play previewed media file, click on preview to play/pause videos or audio

[Elvis Angelaccio]

elvisangelaccio requested changes to this revision.
elvisangelaccio added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> informationpanel.cpp:218-224
> +    else if (action == dateformatAction) {
>          int dateFormat = static_cast<int>(isChecked ? Baloo::DateFormats::ShortFormat : Baloo::DateFormats::LongFormat);
>  
>          InformationPanelSettings::setDateFormat(dateFormat);
>          m_content->refreshMetaData();
>      }
> +    else if (action == previewAutoPlayAction) {

Please move the `else`s at the end of the previous lines.

> phononwidget.cpp:175-188
> +        switch (m_media->state()) {
> +        case Phonon::PlayingState:
> +            if (m_isVideo) {
> +                m_videoPlayer->setToolTip(i18n("Play"));
> +            }
> +            m_media->pause();
> +            break;

Maybe an if-else would be nicer here.

> phononwidget.h:48
>  
> -        void setUrl(const QUrl &url);
> +        void setUrl(const QUrl &url, bool isVideo);
>          QUrl url() const;

bool arguments in API should be avoided in favor of enums.

> pixmapviewer.h:76-77
>  
> +signals:
> +    void clicked();
> +

This signal is a bit redundant. The same goal (tracking left-click events) can be achieved by using an event filter in the phonon widget.

REPOSITORY
  R318 Dolphin

REVISION DETAIL
  https://phabricator.kde.org/D19782

To: meven, #dolphin, elvisangelaccio, ngraham
Cc: ngraham, broulik, kfm-devel, pekkah, alexde, feverfew, meven, spoorun, navarromorales, firef, andrebarros, emmanuelp, mikesomov
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.kde.org/mailman/private/kfm-devel/attachments/20190324/9b359404/attachment.htm>