Skip to content

Commit

Permalink
Merge pull request #25932 from code-dot-org/dance-party-fix-song-can-…
Browse files Browse the repository at this point in the history
…be-changed-while-running

Dance party: Don't allow song to be changed while running
  • Loading branch information
breville committed Nov 9, 2018
2 parents f4d2dab + 99cf67f commit e543667
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 7 deletions.
9 changes: 5 additions & 4 deletions apps/src/dance/Dance.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import Sounds from '../Sounds';
import {TestResults} from '../constants';
import {DanceParty} from '@code-dot-org/dance-party';
import danceMsg from './locale';
import {reducers, setSelectedSong, setSongData} from './redux';
import {reducers, setSelectedSong, setSongData, setRunIsStarting} from './redux';
import trackEvent from '../util/trackEvent';
import {SignInState} from '../code-studio/progressRedux';
import logToCloud from '../logToCloud';
Expand Down Expand Up @@ -381,16 +381,17 @@ Dance.prototype.runButtonClick = async function () {
// Block re-entrancy since starting a run is async
// (not strictly needed since we disable the run button,
// but better to be safe)
if (this.runIsStarting) {
if (getStore().getState().songs.runIsStarting) {
return;
}

// Disable the run button now to give some visual feedback
// that the button was pressed. toggleRunReset() will
// eventually execute down below, but there are some long-running
// tasks that need to complete first
const runButton = document.getElementById('runButton');
runButton.disabled = true;
this.runIsStarting = true;
getStore().dispatch(setRunIsStarting(true));
await this.danceReadyPromise;

//Log song count in Dance Lab
Expand All @@ -404,7 +405,7 @@ Dance.prototype.runButtonClick = async function () {
} finally {
this.studioApp_.toggleRunReset('reset');
// Safe to allow normal run/reset behavior now
this.runIsStarting = false;
getStore().dispatch(setRunIsStarting(false));
}

// Enable the Finish button if is present:
Expand Down
17 changes: 15 additions & 2 deletions apps/src/dance/DanceVisualizationColumn.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ const styles = {

const SongSelector = Radium(class extends React.Component {
static propTypes = {
enableSongSelection: PropTypes.bool,
setSong: PropTypes.func.isRequired,
selectedSong: PropTypes.string,
songData: PropTypes.objectOf(PropTypes.object).isRequired,
Expand All @@ -35,7 +36,13 @@ const SongSelector = Radium(class extends React.Component {
return (
<div>
<label><b>{i18n.selectSong()}</b></label>
<select id="song_selector" style={styles.selectStyle} onChange={this.changeSong} value={this.props.selectedSong}>
<select
id="song_selector"
style={styles.selectStyle}
onChange={this.changeSong}
value={this.props.selectedSong}
disabled={!this.props.enableSongSelection}
>
{Object.keys(this.props.songData).map((option, i) => (
(this.props.filterOff || !this.props.songData[option].pg13) &&
<option key={i} value={option}>{this.props.songData[option].title}</option>
Expand All @@ -52,6 +59,8 @@ class DanceVisualizationColumn extends React.Component {
showFinishButton: PropTypes.bool.isRequired,
setSong: PropTypes.func.isRequired,
selectedSong: PropTypes.string,
levelIsRunning: PropTypes.bool,
levelRunIsStarting: PropTypes.bool,
isShareView: PropTypes.bool.isRequired,
songData: PropTypes.objectOf(PropTypes.object).isRequired,
userType: PropTypes.string.isRequired
Expand All @@ -73,11 +82,13 @@ class DanceVisualizationColumn extends React.Component {
const teacherOverride = queryString.parse(window.location.search).songfilter === 'on';
const signedOutAge = sessionStorage.getItem('anon_over13') ? sessionStorage.getItem('anon_over13') : false;
const filterOff = (signedInOver13 || signedOutAge) && !teacherOverride;
const enableSongSelection = !this.props.levelIsRunning && !this.props.levelRunIsStarting;

return (
<span>
{!this.props.isShareView &&
<SongSelector
enableSongSelection={enableSongSelection}
setSong={this.props.setSong}
selectedSong={this.props.selectedSong}
songData={this.props.songData}
Expand All @@ -103,5 +114,7 @@ export default connect(state => ({
isShareView: state.pageConstants.isShareView,
songData: state.songs.songData,
selectedSong: state.songs.selectedSong,
userType: state.progress.userType
userType: state.progress.userType,
levelIsRunning: state.runState.isRunning,
levelRunIsStarting: state.songs.runIsStarting
}))(DanceVisualizationColumn);
13 changes: 13 additions & 0 deletions apps/src/dance/redux.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
/** @enum {string} */
export const SET_SELECTED_SONG = 'dance/SET_SELECTED_SONG';
export const SET_SONG_DATA = 'dance/SET_SONG_DATA';
export const SET_RUN_IS_STARTING = 'dance/SET_RUN_IS_STARTING';

export function setSelectedSong(song) {
return {
Expand All @@ -18,6 +19,13 @@ export function setSongData(songData) {
};
}

export function setRunIsStarting(runIsStarting) {
return {
type: SET_RUN_IS_STARTING,
runIsStarting
};
}

export const actions = {
setSelectedSong
};
Expand All @@ -42,6 +50,11 @@ function songs(state, action) {
...state,
songData: action.songData,
};
case SET_RUN_IS_STARTING:
return {
...state,
runIsStarting: action.runIsStarting
};
default:
return state;
}
Expand Down
5 changes: 4 additions & 1 deletion dashboard/test/ui/features/spritelab/dancelab.feature
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,15 @@ Feature: Dance Lab
And I close the instructions overlay if it exists
Then element "#runButton" is visible
And element "#resetButton" is hidden
And element "#song_selector" is enabled
Then I click selector "#runButton" once I see it
Then I wait until element "#runButton" is not visible
And element "#resetButton" is visible
And element "#song_selector" is disabled
Then I click selector "#resetButton" once I see it
Then element "#runButton" is visible
And element "#resetButton" is hidden
And element "#song_selector" is enabled

@no_mobile
Scenario: Can get to level success in DanceLab
Expand Down Expand Up @@ -60,7 +63,7 @@ Feature: Dance Lab
Then I wait until element "#runButton" is not visible

Then evaluate JavaScript expression "window.__DanceTestInterface.getSprites().length === 3"

Then I click selector "#resetButton" once I see it
Then element "#runButton" is visible
And element "#resetButton" is hidden

0 comments on commit e543667

Please sign in to comment.