Skip to content
This repository was archived by the owner on Jun 28, 2021. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
removes some unneeded state things in audioplayer redux, also fixes a…
… bug where event listeners would get bound without properly unbinding multiple times, which caused the player to skip ayahs after using next <-> prev more then once on the same ayah
  • Loading branch information
sharabash committed Jun 7, 2016
commit 51f9d2c2ced478fb8d67e053aac09beca8bb4111
15 changes: 9 additions & 6 deletions src/components/Audioplayer/Segments/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import { decrypt } from 'sjcl';
export default class Segments extends Component {
static propTypes = {
audio: PropTypes.object.isRequired,
segments: PropTypes.string.isRequired
segments: PropTypes.string.isRequired,
currentAyah: PropTypes.string,
currentWord: PropTypes.string,
onSetCurrentWord: PropTypes.func.isRequired
};

static defaultProps = {
Expand All @@ -26,12 +29,12 @@ export default class Segments extends Component {

componentDidMount() {
const builtIntervals = this.buildIntervals();
console.debug('Segments componentDidMount', this.props.audio, builtIntervals);
//console.debug('Segments componentDidMount', this.props.audio, builtIntervals);
this.bindListeners();
}

componentWillUnmount() {
console.log('Segments componentWillUnmount', this.props.audio, { props: this.props, state: this.state });
//console.log('Segments componentWillUnmount', this.props.audio, { props: this.props, state: this.state });
this.unbindListeners();
}

Expand Down Expand Up @@ -128,7 +131,7 @@ export default class Segments extends Component {
let repeaterId = null;

new Promise((done, fail) => {
console.debug('Play listener for '+ props.currentAyah +' started...');
//console.debug('Play listener for '+ props.currentAyah +' started...');

const intervalFn = () => {
if (audio.seeking) return console.warn('we are seeking right now?');
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

console.warn will fail on IE. Because there is no console

Expand Down Expand Up @@ -165,7 +168,7 @@ export default class Segments extends Component {
audio.removeEventListener(evName, listeners[evName]);
});

console.debug('Play listener for '+ props.currentAyah +(ev && ev.type ? ' resolved by '+ ev.type : 'stopped') +' event');
//console.debug('Play listener for '+ props.currentAyah +(ev && ev.type ? ' resolved by '+ ev.type : 'stopped') +' event');
});
};
audio.addEventListener('play', play, false);
Expand All @@ -180,7 +183,7 @@ export default class Segments extends Component {
setCurrentWord(currentWord = null, debug = null) {
this.currentWord = currentWord; // this is more immediately available but should eventually agree with props
this.props.onSetCurrentWord(currentWord); // calls the redux dispatch function passed down from the Audioplayer
console.log('setCurrentWord', currentWord, debug ? debug : '');
//console.log('setCurrentWord', currentWord, debug ? debug : '');
}

compareFn(time, interval) {
Expand Down
53 changes: 22 additions & 31 deletions src/components/Audioplayer/Track/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ export default class Track extends Component {
static propTypes = {
file: PropTypes.object.isRequired,
isStarted: PropTypes.bool.isRequired,
isPlaying: PropTypes.bool.isRequired,
doStop: PropTypes.func,
onEnd: PropTypes.func.isRequired,
shouldRepeat: PropTypes.bool.isRequired,
doStop: PropTypes.func,
surah: PropTypes.object
surah: PropTypes.object.isRequired,
currentAyah: PropTypes.string.isRequired
};

state = {
Expand All @@ -25,15 +25,15 @@ export default class Track extends Component {
componentDidMount() {
this.setState({ mounted: true });
if (this.props.file && __CLIENT__) {
console.debug('Track componentDidMount', this.props.file, { file: this.props.file });
//console.debug('Track componentDidMount', this.props.file, { file: this.props.file });
this.onFileLoad(this.props.file);
}
}

componentWillUnmount() {
console.log('Track componentWillUnmount', this.props.file);
this.setState({ mounted: false });
this.state.mounted = false;
//console.log('Track componentWillUnmount', this.props.file);
this.setState({ mounted: false }); // TODO, yeah, this is bad but we unmount and mount when
this.state.mounted = false; // we lazy load, and it causes problems...
// trace memory profile count
this.onFileUnload(this.props.file);
}
Expand All @@ -42,7 +42,6 @@ export default class Track extends Component {
return [
this.props.file.src !== nextProps.file.src,
this.props.isStarted !== nextProps.isStarted,
this.props.isPlaying !== nextProps.isPlaying,
this.props.shouldRepeat !== nextProps.shouldRepeat,
this.state.progress !== nextState.progress, // do we need this??
this.state.currentTime !== nextState.currentTime // do we need this??
Expand All @@ -67,14 +66,20 @@ export default class Track extends Component {
// Preload file
file.setAttribute('preload', 'auto');

/*
if (!this.didAlreadyLoad) this.didAlreadyLoad = {};
if (this.didAlreadyLoad[file.src]) return;
else this.didAlreadyLoad[file.src] = true;
*/

const loadeddata = () => {
// Default current time to zero. This will change
file.currentTime = 0; // eslint-disable-line no-param-reassign
};
file.addEventListener('loadeddata', loadeddata);
file.onloadeddata = loadeddata;

const timeupdate = () => {
console.assert(this.state.mounted, 'timeupdate without being mounted', file, { file, mounted: this.state.mounted });
//console.assert(this.state.mounted, 'timeupdate without being mounted', file, { file, mounted: this.state.mounted });
if (!this.state.mounted) return; // TODO needed?

const progress = (
Expand All @@ -84,7 +89,7 @@ export default class Track extends Component {

this.setState({ progress });
};
file.addEventListener('timeupdate', timeupdate, false);
file.ontimeupdate = timeupdate;

const ended = () => {
const { shouldRepeat, onEnd, isStarted, doStop, currentAyah, surah } = this.props;
Expand All @@ -101,7 +106,7 @@ export default class Track extends Component {
onEnd();
}
};
file.addEventListener('ended', ended, false);
file.onended = ended;

const play = () => {
if (!this.state.mounted) return;
Expand All @@ -111,28 +116,14 @@ export default class Track extends Component {

this.setState({ currentTime });
};
file.addEventListener('play', play, false);

this.setState({
listeners: {
loadeddata,
timeupdate,
ended,
play
}
});
file.onplay = play;
}

onFileUnload(file) {
//console.warn('onFileUnload');
if (!file.paused) {
file.pause();
}

setTimeout(() => {
[ 'loadeddata', 'timeupdate', 'ended', 'play' ].forEach((listener) => {
file.removeEventListener(listener, this.state.listeners[listener]);
});
}, 50);
}

onTrackerMove(event) {
Expand All @@ -155,14 +146,14 @@ export default class Track extends Component {

render() {
const { progress, mounted } = this.state;
const { isPlaying, file } = this.props;
const { isStarted, file } = this.props;

if (file.readyState >= 3) {
// the Math.round bit prevents us from trying to play again when we're effectively at the end of the audio file; this should allow shouldRepeat to work without getting overridden:
// ...but at the time I monkey-patched it, so we might be able to get rid of it since we cleaned up? Let's not for now...
if (isPlaying && file.paused && file.readyState >= 3 && Math.round(file.currentTime) != Math.round(file.duration)) {
if (isStarted && file.paused && file.readyState >= 3 && Math.round(file.currentTime) != Math.round(file.duration)) {
file.play();
} else if (!isPlaying && !file.paused) {
} else if (!isStarted && !file.paused) {
file.pause();
}
}
Expand Down
26 changes: 12 additions & 14 deletions src/components/Audioplayer/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ const style = require('./style.scss');
surahId: state.audioplayer.surahId,
isSupported: state.audioplayer.isSupported,
isStarted: state.audioplayer.isStarted,
isPlaying: state.audioplayer.isPlaying,
isLoadedOnClient: state.audioplayer.isLoadedOnClient,
shouldRepeat: state.audioplayer.shouldRepeat,
shouldScroll: state.audioplayer.shouldScroll
Expand Down Expand Up @@ -70,7 +69,6 @@ export default class Audioplayer extends Component {
currentAyah: PropTypes.string,
currentWord: PropTypes.string,
buildOnClient: PropTypes.func.isRequired,
isPlaying: PropTypes.bool.isRequired,
isLoadedOnClient: PropTypes.bool.isRequired,
isSupported: PropTypes.bool.isRequired,
shouldRepeat: PropTypes.bool.isRequired,
Expand Down Expand Up @@ -99,15 +97,15 @@ export default class Audioplayer extends Component {
debug('component:Audioplayer', 'componentDidMount');

if (!isLoadedOnClient && __CLIENT__) {
console.debug('Audioplayer componentDidMount');
//console.debug('Audioplayer componentDidMount');
debug('component:Audioplayer', 'componentDidMount on client');
return buildOnClient(surah.id);
} else console.debug('Audioplayer componentDidMount', { notLoadedOnClient: !isLoadedOnClient, client: __CLIENT__ });
}// else console.debug('Audioplayer componentDidMount', { notLoadedOnClient: !isLoadedOnClient, client: __CLIENT__ });
}

componentWillUnmount() {
debug('component:Audioplayer', 'componentWillUnmount');
console.log('Audioplayer componentWillUnmount');
//console.log('Audioplayer componentWillUnmount');
this.stop();
}

Expand Down Expand Up @@ -145,6 +143,9 @@ export default class Audioplayer extends Component {

onNextAyah() {
const { setCurrentAyah, isStarted, shouldScroll } = this.props; // eslint-disable-line no-shadow

const file = this.props.files[this.props.currentAyah];

const nextAyah = this.getNext();
if (!nextAyah) return this.stop();
const ayahNum = nextAyah.replace( /^\d+:/, '' );
Expand Down Expand Up @@ -172,7 +173,7 @@ export default class Audioplayer extends Component {
// the previous button
const { currentAyah, ayahIds } = this.props;
const index = ayahIds.findIndex(id => id === currentAyah) - 1;
console.debug('getPrevious', { props: this.props, index, prevAyah: ayahIds[index], currentAyah })
//console.debug('getPrevious', { props: this.props, index, prevAyah: ayahIds[index], currentAyah })
return ayahIds[index];
}

Expand All @@ -184,7 +185,7 @@ export default class Audioplayer extends Component {
onLoadAyahs(); // this doesnt look right, should probably be returned or promise.then?
}

console.debug('getNext', { props: this.props, index, nextAyah: ayahIds[index], currentAyah })
//console.debug('getNext', { props: this.props, index, nextAyah: ayahIds[index], currentAyah })
return ayahIds[index];
}

Expand Down Expand Up @@ -358,7 +359,6 @@ export default class Audioplayer extends Component {
currentAyah,
currentWord,
setCurrentWord,
isPlaying,
isStarted,
shouldRepeat,
isSupported,
Expand Down Expand Up @@ -407,15 +407,13 @@ export default class Audioplayer extends Component {
<div className={style.wrapper}>
{isLoadedOnClient ?
<Track
// TODO verify what is used and isnt
file={files[currentAyah]}
isStarted={isStarted}
isPlaying={isPlaying}
currentAyah={currentAyah}
surah={surah}
shouldRepeat={shouldRepeat}
onEnd={this.onNextAyah.bind(this)}
doStop={this.stop.bind(this)}
onEnd={this.onNextAyah.bind(this)}
shouldRepeat={shouldRepeat}
surah={surah}
currentAyah={currentAyah}
/> : null}
{isLoadedOnClient && segments[currentAyah] ?
<Segments
Expand Down
2 changes: 0 additions & 2 deletions src/containers/Surah/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -109,13 +109,11 @@ const ayahRangeSize = 30;

const currentWord = state.ayahs.currentWord;
const currentAyah = state.ayahs.currentAyah;
const isPlaying = state.audioplayer.isPlaying;
const isStarted = state.audioplayer.isStarted;
const isEndOfSurah = ayahIds.last() === surah.ayat;

return {
isStarted,
isPlaying,
currentWord,
currentAyah,
surah,
Expand Down
48 changes: 3 additions & 45 deletions src/redux/modules/audioplayer.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ const SET_USER_AGENT = '@@quran/audioplayer/SET_USER_AGENT';
const SET_CURRENT_FILE = '@@quran/audioplayer/SET_CURRENT_FILE';
const START = '@@quran/audioplayer/START';
const STOP = '@@quran/audioplayer/STOP';
const PLAY = '@@quran/audioplayer/PLAY';
const PAUSE = '@@quran/audioplayer/PAUSE';
const PLAY_PAUSE = '@@quran/audioplayer/PLAY_PAUSE';
const TOGGLE_REPEAT = '@@quran/audioplayer/TOGGLE_REPEAT';
const TOGGLE_SCROLL = '@@quran/audioplayer/TOGGLE_SCROLL';
const BUILD_ON_CLIENT = '@@quran/audioplayer/BUILD_ON_CLIENT';
Expand All @@ -20,8 +17,7 @@ const initialState = {
userAgent: null,
currentFile: null,
isSupported: true,
isPlaying: false,
isStarted: false, // like isPlaying, but doesn't toggle off everytime we have a brief pause between ayah transition
isStarted: false,
shouldRepeat: false,
shouldScroll: false,
isLoadedOnClient: false
Expand Down Expand Up @@ -115,33 +111,13 @@ export default function reducer(state = initialState, action = {}) {
console.debug('START');
return {
...state,
isStarted: true,
isPlaying: true
isStarted: true
};
case STOP:
console.debug('STOP');
return {
...state,
isStarted: false,
isPlaying: false
};
case PLAY:
console.debug('DISPATCH PLAY');
return {
...state,
isPlaying: true
};
case PAUSE:
console.debug('DISPATCH PAUSE');
return {
...state,
isPlaying: false
};
case PLAY_PAUSE:
console.debug('PLAY_PAUSE');
return {
...state,
isPlaying: !state.isPlaying
isStarted: false
};
case TOGGLE_REPEAT:
return {
Expand Down Expand Up @@ -194,24 +170,6 @@ export function stop() {
};
}

export function play() {
return {
type: PLAY
};
}

export function pause() {
return {
type: PAUSE
};
}

export function playPause() {
return {
type: PLAY_PAUSE
};
}

export function toggleRepeat() {
return {
type: TOGGLE_REPEAT
Expand Down