From 4c555a4887b82cfabaa90fbbc214e90b674b85a9 Mon Sep 17 00:00:00 2001 From: Jordan Eldredge Date: Tue, 20 Feb 2018 19:43:20 -0800 Subject: [PATCH] Improve track display name fallback. --- js/actionCreators.js | 18 ++--------- js/fileUtils.js | 7 ++++ js/reducers/playlist.js | 12 +++++-- js/reducers/playlist.test.js | 62 ++++++++++++++++++++++++++++++++---- 4 files changed, 74 insertions(+), 25 deletions(-) diff --git a/js/actionCreators.js b/js/actionCreators.js index 821a89ef..7fbee50e 100644 --- a/js/actionCreators.js +++ b/js/actionCreators.js @@ -3,8 +3,7 @@ import { parser, creator } from "winamp-eqf"; import { genArrayBufferFromFileReference, genArrayBufferFromUrl, - promptForFileReferences, - filenameFromUrl + promptForFileReferences } from "./fileUtils"; import skinParser from "./skinParser"; import { BANDS, TRACK_HEIGHT, LOAD_STYLE } from "./constants"; @@ -253,15 +252,6 @@ function uniqueId() { return counter++; } -/* -type Track = { - defaultName: ?string, - duration: ?number, - url?: string, - blob?: fileReference -} -*/ - export function loadMediaFiles(tracks, loadStyle = null, atIndex = 0) { return dispatch => { if (loadStyle === LOAD_STYLE.PLAY) { @@ -281,20 +271,16 @@ export function loadMediaFile(track, priority = null, atIndex = 0) { const id = uniqueId(); const { url, blob, defaultName, metaData, duration } = track; let canonicalUrl = url; - let canonicalDefaultName = defaultName; if (canonicalUrl == null) { if (blob == null) { throw new Error("Expected track to have either a blob or a url"); } canonicalUrl = URL.createObjectURL(track.blob); - } else if (!defaultName) { - canonicalDefaultName = filenameFromUrl(url); - // TODO: Derive the defaultName from the URL if none is provided } dispatch({ type: ADD_TRACK_FROM_URL, url: canonicalUrl, - name: canonicalDefaultName, + defaultName, id, atIndex }); diff --git a/js/fileUtils.js b/js/fileUtils.js index 4df5db70..77f1986c 100644 --- a/js/fileUtils.js +++ b/js/fileUtils.js @@ -48,8 +48,15 @@ export async function promptForFileReferences(accept) { }); } +function urlIsBlobUrl(url) { + return /^blob:/.test(url); +} + // This is not perfect, but... meh: https://stackoverflow.com/a/36756650/1263117 export function filenameFromUrl(url) { + if (urlIsBlobUrl(url)) { + return null; + } return url .split("/") .pop() diff --git a/js/reducers/playlist.js b/js/reducers/playlist.js index badd20ce..918f7ca1 100644 --- a/js/reducers/playlist.js +++ b/js/reducers/playlist.js @@ -19,6 +19,7 @@ import { SET_MEDIA_DURATION } from "../actionTypes"; +import { filenameFromUrl } from "../fileUtils"; import { shuffle, moveSelected, mapObject, filterObject } from "../utils"; const defaultPlaylistState = { @@ -145,7 +146,7 @@ const playlist = (state = defaultPlaylistState, action) => { ...state.tracks, [action.id]: { selected: false, - title: action.name, + defaultName: action.defaultName, duration: null, url: action.url } @@ -216,11 +217,18 @@ export const getTrackDisplayName = (state, id) => { if (track == null) { return null; } - const { artist, title } = track; + const { artist, title, defaultName, url } = track; if (artist && title) { return `${artist} - ${title}`; } else if (title) { return title; + } else if (defaultName) { + return defaultName; + } else if (url) { + const filename = filenameFromUrl(url); + if (filename) { + return filename; + } } return "???"; }; diff --git a/js/reducers/playlist.test.js b/js/reducers/playlist.test.js index a7a7a981..6a8fbbaf 100644 --- a/js/reducers/playlist.test.js +++ b/js/reducers/playlist.test.js @@ -4,7 +4,7 @@ import { CTRL_CLICKED_TRACK, ADD_TRACK_FROM_URL } from "../actionTypes"; -import reducer from "./playlist"; +import reducer, { getTrackDisplayName } from "./playlist"; describe("playlist reducer", () => { it("can handle adding a track", () => { @@ -16,7 +16,7 @@ describe("playlist reducer", () => { const nextState = reducer(initialState, { type: ADD_TRACK_FROM_URL, id: 100, - name: "My Track Name", + defaultName: "My Track Name", url: "url://some-url" }); expect(nextState).toEqual({ @@ -24,7 +24,7 @@ describe("playlist reducer", () => { 100: { selected: false, duration: null, - title: "My Track Name", + defaultName: "My Track Name", url: "url://some-url" } }, @@ -44,7 +44,7 @@ describe("playlist reducer", () => { const nextState = reducer(initialState, { type: ADD_TRACK_FROM_URL, id: 100, - name: "My Track Name", + defaultName: "My Track Name", url: "url://some-url" }); expect(nextState).toEqual({ @@ -54,7 +54,7 @@ describe("playlist reducer", () => { 100: { selected: false, duration: null, - title: "My Track Name", + defaultName: "My Track Name", url: "url://some-url" } }, @@ -74,7 +74,7 @@ describe("playlist reducer", () => { const nextState = reducer(initialState, { type: ADD_TRACK_FROM_URL, id: 100, - name: "My Track Name", + defaultName: "My Track Name", url: "url://some-url", atIndex: 1 }); @@ -85,7 +85,7 @@ describe("playlist reducer", () => { 100: { selected: false, duration: null, - title: "My Track Name", + defaultName: "My Track Name", url: "url://some-url" } }, @@ -167,3 +167,51 @@ describe("playlist reducer", () => { }); }); }); + +describe("getTrackDisplayName", () => { + const expectDisplayName = (track, expected) => { + expect(getTrackDisplayName({ tracks: { "1": track } }, "1")).toBe(expected); + }; + it("uses the artists and title if provided", () => { + expectDisplayName( + { + artist: "Artist", + title: "Title", + defaultName: "Default Name", + url: "https://example.com/dir/filename.mp3" + }, + "Artist - Title" + ); + }); + it("uses the title if provided", () => { + expectDisplayName( + { + title: "Title", + defaultName: "Default Name", + url: "https://example.com/dir/filename.mp3" + }, + "Title" + ); + }); + it("uses a defaultName if provided", () => { + expectDisplayName( + { + defaultName: "Default Name", + url: "https://example.com/dir/filename.mp3" + }, + "Default Name" + ); + }); + it("uses the filename if a URL is provided", () => { + expectDisplayName( + { url: "https://example.com/dir/filename.mp3" }, + "filename.mp3" + ); + }); + it("does not use the filename if a blob URL is provided", () => { + expectDisplayName({ url: "blob:foo" }, "???"); + }); + it("falls back to '???'", () => { + expectDisplayName({}, "???"); + }); +});