Differential D1106

Fix for T1793
ClosedPublic
Actions

Authored by itsback on Mar 10 2016, 3:46 PM.

Details

Reviewers

sandroandrade

Maniphest Tasks

T1793: Merge 'Play' and 'Pause' button in MidiPlayer

Commits

R163:cdc66fce54c8: Merge the play and pause button

Summary

This differential merges the play and pause button

Test Plan

Instead of having two multimedia buttons, we can have the functionality with only one button with two states.
One state for play and other for pause.
The state can change if one of the three functions are called

void MidiSequencer::play()
void MidiSequencer::pause()
void MidiSequencer::stop()

Hence whenever they are called, a signal is sent to slot in MidiPlayer.qml.

Diff Detail

Repository

R163 Minuet

Lint

Automatic diff as part of commit; lint not applicable.

Unit

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

itsback updated this revision to Diff 2666.Mar 10 2016, 3:46 PM

itsback retitled this revision from to Fix for T1793.

itsback updated this object.

itsback edited the test plan for this revision. (Show Details)

itsback added a reviewer: sandroandrade.

itsback set the repository for this revision to R163 Minuet.

itsback added a project: Minuet.

itsback added a subscriber: itsback.

itsback added a task: T1793: Merge 'Play' and 'Pause' button in MidiPlayer.Mar 10 2016, 3:56 PM

The button now changes back to 'Play' state after playback is finished.

Hi,

Please make the suggested changes and submit a new patch in this same Differential D1106.
Also, please rebase your changes against current master state, since new commits have arrived since then.

Thanks,

src/midisequencer.cpp
183	Please emit stateChanged(MidiSequencer::StoppedState) here and act accordingly in QML files.
194	Please emit stateChanged(MidiSequencer::PlayingState) here.
205	Please emit stateChanged(MidiSequencer::PausedState) here.
214	Please emit stateChanged(MidiSequencer::StoppedState) here.
src/midisequencer.h
67	Please name this method 'stateChanged'. Also, create an enum named 'State' with values: StoppedState, PlayingState, and PausedState and use this enum as signal parameter.

This revision now requires changes to proceed.Mar 15 2016, 1:47 AM

sandroandrade accepted this revision.Mar 15 2016, 1:58 AM

sandroandrade edited edge metadata.

This comment was removed by sandroandrade.

This revision is now accepted and ready to land.Mar 15 2016, 1:58 AM

sandroandrade requested changes to this revision.Mar 15 2016, 2:25 AM

sandroandrade edited edge metadata.

This revision now requires changes to proceed.Mar 15 2016, 2:25 AM

Should the states in State enum be in all caps?
like STOPPED_STATE instead of StoppedState?

sandroandrade accepted this revision.May 26 2016, 7:18 PM

sandroandrade edited edge metadata.

This revision is now accepted and ready to land.May 26 2016, 7:18 PM

Closed by commit R163:cdc66fce54c8: Merge the play and pause button (authored by itsback, committed by sandroandrade). · Explain WhyMay 26 2016, 7:22 PM

This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

			Path	Packages
M			src/midisequencer.h (9 lines)
M			src/midisequencer.cpp (11 lines)
M			src/qml/Main.qml (1 line)
M			src/qml/midiplayer/MidiPlayer.qml (44 lines)

Diff	ID	Base	Description	Created	Lint	Unit
Base			Base
Diff 1	2666			Mar 10 2016, 3:39 PM	★	★
Diff 2	2675			Mar 10 2016, 7:52 PM	★	★
Diff 3	2841			Mar 19 2016, 6:07 AM	★	★
Diff 4	4018	394053f	rMINUETcdc66fce54c8ccf15e57fc78bae88f41600b23f1	May 26 2016, 7:21 PM	★	★