Align slideshow terminology to MPRIS behaviour
ClosedPublic
Actions

Authored by rkflx on Mar 19 2018, 5:02 PM.

Details

Reviewers

kossebau
muhlenpfordt

Group Reviewers

Gwenview

Maniphest Tasks

T8222: Improve MPRIS support

Commits

R260:beef8f3b0f6f: Align slideshow terminology to MPRIS behaviour

Summary

9631043c110d introduced MPRIS support to Gwenview. Common MPRIS
controllers often only provide "Pause" in the main interface, with
"Stop" being non-existent or hidden. Therefore "holding" the slideshow
was mapped to "Pause". "Play" either "Resumes" automatically advancing
to the next image, or in non-fullscreen mode switches to fullscreen and
starts playback. Conversely, "Stop" ends playback and exits from
fullscreen.

To keep consistency, we change the tooltip text of the fullscreen button
controlling the playback to "Pause". In addition, the internal enum is
adapted accordingly to avoid confusing developers.

Note that the icon on the button still shows a "Pause" symbol as before
the patch.

Ref T8222

Test Plan

Start Gwenview normally, with -f and with -s, and observe the
tooltip of the playback button in the top fullscreen toolbar as well as
the button text in non-fullscreen mode (use Configure Toolbars to
add it) make sense in every situation.

Diff Detail

Repository

R260 Gwenview

Lint

Automatic diff as part of commit; lint not applicable.

Unit

Automatic diff as part of commit; unit tests not applicable.

rkflx created this revision.Mar 19 2018, 5:02 PM

Restricted Application added a subscriber: kde-doc-english. · View Herald TranscriptMar 19 2018, 5:02 PM

rkflx requested review of this revision.Mar 19 2018, 5:02 PM

rkflx mentioned this in D11492: Prevent duplicate slashes in Recent Folders.Mar 21 2018, 1:05 AM

For what it's worth, this change makes sense to me, and I have tested it with no percieved issues.

The naming looks good to me.
Menu/button always displays Start Slideshow in normal mode and after switching to fullscreen. Only after starting/pausing a slideshow Pause / Resume Slideshow is displayed.

This revision is now accepted and ready to land.Mar 21 2018, 9:04 AM

A note about the deadlines: this can still land onto the newly created Applications/18.04, at least until the freeze tomorrow.

In D11495#230603, @ltoscano wrote:

A note about the deadlines: this can still land onto the newly created Applications/18.04, at least until the freeze tomorrow.

Thanks Luigi, we are aware of that ;)

Closed by commit R260:beef8f3b0f6f: Align slideshow terminology to MPRIS behaviour (authored by rkflx). · Explain WhyMar 21 2018, 9:27 AM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			app/mainwindow.cpp (9 lines)
M			doc/index.docbook (2 lines)
M			lib/mpris2/mprismediaplayer2player.cpp (2 lines)
M			lib/slideshow.h (4 lines)
M			lib/slideshow.cpp (16 lines)