Improve track display name fallback.

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
     });

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()

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 "???";
 };

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({}, "???");
+  });
+});