diff --git a/js/components/PlaylistWindow/TrackCell.js b/js/components/PlaylistWindow/TrackCell.js index abe027fd..d314d278 100644 --- a/js/components/PlaylistWindow/TrackCell.js +++ b/js/components/PlaylistWindow/TrackCell.js @@ -6,6 +6,7 @@ import { CTRL_CLICKED_TRACK, PLAY_TRACK } from "../../actionTypes"; +import { getCurrentTrackId } from "../../selectors"; const TrackCell = props => { const { @@ -35,17 +36,10 @@ const TrackCell = props => { }; const mapStateToProps = (state, ownProps) => { - const { - display: { skinPlaylistStyle }, - playlist: { currentTrack }, - tracks - } = state; + const { display: { skinPlaylistStyle }, tracks } = state; + const current = getCurrentTrackId(state) === ownProps.id; const track = tracks[ownProps.id]; - return { - skinPlaylistStyle, - selected: track.selected, - current: currentTrack === ownProps.id - }; + return { skinPlaylistStyle, selected: track.selected, current }; }; const mapDispatchToProps = (dispatch, ownProps) => ({ diff --git a/js/mediaMiddleware.js b/js/mediaMiddleware.js index e644614c..f6f7bd9d 100644 --- a/js/mediaMiddleware.js +++ b/js/mediaMiddleware.js @@ -18,6 +18,7 @@ import { PLAY_TRACK } from "./actionTypes"; import { next as nextTrack } from "./actionCreators"; +import { getCurrentTrackId } from "./selectors"; export default media => store => { media.addEventListener("timeupdate", () => { @@ -52,7 +53,7 @@ export default media => store => { channels: media.channels(), name: media.name, length: media.duration(), - id: store.getState().playlist.currentTrack + id: getCurrentTrackId(store.getState()) }); }); diff --git a/js/reducers.js b/js/reducers.js index 342263fa..50f08756 100644 --- a/js/reducers.js +++ b/js/reducers.js @@ -275,13 +275,15 @@ const tracks = (state = defaultTracksState, action) => { const defaultPlaylistState = { trackOrder: [], - currentTrack: null + currentTrackIndex: null }; const playlist = (state = defaultPlaylistState, action) => { switch (action.type) { case REMOVE_ALL_TRACKS: - return { ...state, trackOrder: [], currentTrack: null }; + // TODO: Consider disposing of ObjectUrls + return { ...state, trackOrder: [], currentTrackIndex: null }; case REMOVE_TRACKS: + // TODO: Consider disposing of ObjectUrls return { ...state, trackOrder: state.trackOrder.filter(id => !action.ids.includes(id)) @@ -303,12 +305,12 @@ const playlist = (state = defaultPlaylistState, action) => { return { ...state, trackOrder: [...state.trackOrder, action.id], - currentTrack: action.id + currentTrackIndex: state.trackOrder.length }; case PLAY_TRACK: return { ...state, - currentTrack: action.id + currentTrackIndex: state.trackOrder.findIndex(id => id === action.id) }; default: return state; diff --git a/js/selectors.js b/js/selectors.js index 70a8fb62..741d2942 100644 --- a/js/selectors.js +++ b/js/selectors.js @@ -58,14 +58,18 @@ export const getRunningTimeMessage = createSelector( `${getTimeStr(selectedRunningTime)}/${getTimeStr(totalRunningTime)}` ); -// TODO: Consider changing `currentTrack` in the sate to be this value and -// compute the other way (cheaper) -export const getCurrentTrackIndex = state => { - const { trackOrder, currentTrack } = state.playlist; - return trackOrder.findIndex(trackId => trackId === currentTrack); -}; +export const getCurrentTrackIndex = state => state.playlist.currentTrackIndex; -export const getCurrentTrackNumber = state => getCurrentTrackIndex(state) + 1; +export const getCurrentTrackNumber = createSelector( + getCurrentTrackIndex, + currentTrackIndex => currentTrackIndex + 1 +); + +export const getCurrentTrackId = createSelector( + getTrackOrder, + getCurrentTrackIndex, + (trackOrder, currentTrackIndex) => trackOrder[currentTrackIndex] +); export const nextTrack = (state, n = 1) => { const { playlist: { trackOrder }, media: { repeat } } = state; diff --git a/js/selectors.test.js b/js/selectors.test.js index 639b5c67..9a816ff1 100644 --- a/js/selectors.test.js +++ b/js/selectors.test.js @@ -31,7 +31,7 @@ describe("getEqfData", () => { describe("nextTrack", () => { it("returns null if you don't have any tracks", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: false } }; expect(state.playlist.trackOrder).toEqual([]); @@ -40,81 +40,81 @@ describe("nextTrack", () => { it("returns null if you are going forward from the last track and repeat is not turned on", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: false } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 3; + state.playlist.currentTrackIndex = 2; expect(nextTrack(state)).toBe(null); }); it("wraps around if you are going forward from the last track and repeat _is_ turned on", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: true } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 3; + state.playlist.currentTrackIndex = 2; expect(nextTrack(state)).toBe(1); }); it("returns null if you are going backward from the first track and repeat is not turned on", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: false } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 1; + state.playlist.currentTrackIndex = 0; expect(nextTrack(state, -1)).toBe(null); }); it("wraps around if you are going backwards from the first track and repeat _is_ turned on", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: true } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 1; + state.playlist.currentTrackIndex = 0; expect(nextTrack(state, -1)).toBe(3); }); it("does a normal next", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: false } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 2; + state.playlist.currentTrackIndex = 1; expect(nextTrack(state)).toBe(3); }); it("does a normal previous", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: false } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 2; + state.playlist.currentTrackIndex = 1; expect(nextTrack(state, -1)).toBe(1); }); it("takes you to the last track if you overshoot", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: false } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 2; + state.playlist.currentTrackIndex = 1; expect(nextTrack(state, 10)).toBe(3); }); it("takes you to the first track if you overshoot", () => { const state = { - playlist: { currentTrack: null, trackOrder: [] }, + playlist: { currentTrackIndex: null, trackOrder: [] }, media: { repeat: false } }; state.playlist.trackOrder = [1, 2, 3]; - state.playlist.currentTrack = 2; + state.playlist.currentTrackIndex = 1; expect(nextTrack(state, -10)).toBe(1); }); });