Skip to content

Commit

Permalink
LB-778: Don't clear add-to-playlist search input on blur (#1254)
Browse files Browse the repository at this point in the history
* LB-778: Don't clear add-to-playlist search input on blur

The solution is: cache everything!
Cache the search query and prevent it from being cleared, and cache the ACRM results for that query too.
Also don't close the results on select so that users can add multiple songs easily.

* Playlist search dropdonw: handle 'input-blur' event

* Playlist page: add test for search select input blur

* Playlist page tests: reproducible snapshot

Fix a quirk with fontawesome hash ids that make the snapshots different each time.

* Set timezone to GMT when running tests

To solve issue with snapshots and date string formatted with toLocaleString.

* Update Playlist test snapshot using GMT

Using the previous commit's TZ change

* Mock timeago in Playlist page tests
  • Loading branch information
MonkeyDo committed Feb 10, 2021
1 parent 5d22181 commit bdbd055
Show file tree
Hide file tree
Showing 5 changed files with 155 additions and 17 deletions.
@@ -0,0 +1,39 @@
{
"currentUser": {
"id": 1,
"name": "iliekcomputers"
},
"playlist": {
"playlist": {
"creator": "iliekcomputers",
"identifier": "https://listenbrainz.org/playlist/4245ccd3-4f0d-4276-95d6-2e09d87b5546",
"date": "2021-01-04T14:43:58.359215+00:00",
"title": "1980s flashback jams",
"annotation": "Your lame <i>80s</i> music",
"extension": {
"https://musicbrainz.org/doc/jspf#playlist": {
"public": true,
"last_modified_at": "2021-01-18T16:44:22.837342+00:00"
}
},
"track": [
{
"creator": "Endless Boogie",
"extension": {
"https://musicbrainz.org/doc/jspf#track": {
"added_at": "2020-12-24T01:41:44.821939+00:00",
"added_by": "troi-bot",
"artist_identifier": [
"https://musicbrainz.org/artist/bded3a41-acd7-42ff-9402-cb59e35be8f4"
]
}
},
"identifier": "https://musicbrainz.org/recording/385f4381-0f6d-47a0-bc56-07570374f650",
"title": "Vibe Killer"
}
]
}
},
"webSocketsServerUrl": "http://localhost:8081",
"apiUrl": "http://0.0.0.0"
}
73 changes: 73 additions & 0 deletions listenbrainz/webserver/static/js/src/playlists/Playlist.test.tsx
@@ -0,0 +1,73 @@
import { enableFetchMocks } from "jest-fetch-mock";
import * as React from "react";
import { shallow } from "enzyme";
import * as timeago from "time-ago";
import PlaylistPage from "./Playlist";
import * as playlistPageProps from "../__mocks__/playlistPageProps.json";

enableFetchMocks();

// Font Awesome generates a random hash ID for each icon everytime.
// Mocking Math.random() fixes this
// https://github.com/FortAwesome/react-fontawesome/issues/194#issuecomment-627235075
jest.spyOn(global.Math, "random").mockImplementation(() => 0);

const {
apiUrl,
currentUser,
playlist,
webSocketsServerUrl,
} = playlistPageProps;

const props = {
apiUrl,
playlist: playlist as JSPFObject,
spotify: {
access_token: "heyo",
permission: "streaming" as SpotifyPermission,
},
currentUser,
webSocketsServerUrl,
};

describe("PlaylistPage", () => {
it("renders correctly", () => {
// Mock timeago (returns an elapsed time string) otherwise snapshot won't match
timeago.ago = jest.fn().mockImplementation(() => "1 day ago");
const wrapper = shallow<PlaylistPage>(<PlaylistPage {...props} />);
expect(wrapper.html()).toMatchSnapshot();
});
it("does not clear the add-a-track input on blur", async () => {
const wrapper = shallow<PlaylistPage>(<PlaylistPage {...props} />);
const instance = wrapper.instance();
let searchInput = wrapper.find(".search");
expect(instance.state.searchInputValue).toEqual("");
// @ts-ignore
expect(searchInput.props().inputValue).toEqual("");

searchInput.simulate("focus");
instance.handleInputChange("mysearch", { action: "input-change" });

expect(instance.state.searchInputValue).toEqual("mysearch");
searchInput = wrapper.find(".search");
// @ts-ignore
expect(searchInput.props().inputValue).toEqual("mysearch");

// simulate ReactSelect input blur event
searchInput.simulate("blur");
instance.handleInputChange("", { action: "input-blur" });

searchInput = wrapper.find(".search");
expect(instance.state.searchInputValue).toEqual("mysearch");
// @ts-ignore
expect(searchInput.props().inputValue).toEqual("mysearch");

// simulate ReactSelect menu close event (blur)
instance.handleInputChange("", { action: "menu-close" });

searchInput = wrapper.find(".search");
expect(instance.state.searchInputValue).toEqual("mysearch");
// @ts-ignore
expect(searchInput.props().inputValue).toEqual("mysearch");
});
});
45 changes: 34 additions & 11 deletions listenbrainz/webserver/static/js/src/playlists/Playlist.tsx
Expand Up @@ -2,10 +2,10 @@

import * as React from "react";
import * as ReactDOM from "react-dom";
import { isEqual, get, findIndex, omit, isNil, has } from "lodash";
import { get, findIndex, omit, isNil, has } from "lodash";
import * as io from "socket.io-client";

import { ActionMeta, ValueType } from "react-select";
import { ActionMeta, InputActionMeta, ValueType } from "react-select";
import {
faCog,
faPen,
Expand Down Expand Up @@ -55,6 +55,8 @@ export interface PlaylistPageState {
playlist: JSPFPlaylist;
recordingFeedbackMap: RecordingFeedbackMap;
loading: boolean;
searchInputValue: string;
cachedSearchResults: OptionType[];
}

type OptionType = { label: string; value: ACRMSearchResult };
Expand All @@ -76,7 +78,6 @@ export default class PlaylistPage extends React.Component<
private spotifyPlaylist?: SpotifyPlaylistObject;
private searchForTrackDebounced: any;
private brainzPlayer = React.createRef<BrainzPlayer>();
private addTrackSelectRef = React.createRef<AsyncSelect<OptionType>>();

private socket!: SocketIOClient.Socket;

Expand All @@ -96,6 +97,8 @@ export default class PlaylistPage extends React.Component<
playlist: props.playlist?.playlist || {},
recordingFeedbackMap: {},
loading: false,
searchInputValue: "",
cachedSearchResults: [],
};

this.APIService = new APIService(
Expand Down Expand Up @@ -187,11 +190,6 @@ export default class PlaylistPage extends React.Component<
getPlaylistId(playlist),
[jspfTrack]
);
if (this.addTrackSelectRef?.current?.select) {
(this.addTrackSelectRef.current.select as any).setState({
value: null,
});
}
this.newAlert("success", "Added track", `Added track ${label}`);
/* Deactivating feedback until the feedback system works with MBIDs instead of MSIDs */
/* const recordingFeedbackMap = await this.loadFeedback([
Expand All @@ -206,6 +204,9 @@ export default class PlaylistPage extends React.Component<
this.handleError(error);
}
}
if (actionMeta.action === "clear") {
this.setState({ searchInputValue: "", cachedSearchResults: [] });
}
};

searchForTrack = async (inputValue: string): Promise<OptionType[]> => {
Expand All @@ -223,10 +224,12 @@ export default class PlaylistPage extends React.Component<
// Converting to JSON
const parsedResponse: ACRMSearchResult[] = await response.json();
// Format the received items to a react-select option
return parsedResponse.map((hit: ACRMSearchResult) => ({
const results = parsedResponse.map((hit: ACRMSearchResult) => ({
label: `${hit.recording_name}${hit.artist_credit_name}`,
value: hit,
}));
this.setState({ cachedSearchResults: results });
return results;
} catch (error) {
// eslint-disable-next-line no-console
console.debug(error);
Expand Down Expand Up @@ -681,8 +684,25 @@ export default class PlaylistPage extends React.Component<
this.setState({ loading: false });
};

handleInputChange = (inputValue: string, params: InputActionMeta) => {
/* Prevent clearing the search value on select dropdown close and input blur */
if (["menu-close", "set-value", "input-blur"].includes(params.action)) {
const { searchInputValue } = this.state;
this.setState({ searchInputValue });
} else {
this.setState({ searchInputValue: inputValue, cachedSearchResults: [] });
}
};

render() {
const { alerts, currentTrack, playlist, loading } = this.state;
const {
alerts,
currentTrack,
playlist,
loading,
searchInputValue,
cachedSearchResults,
} = this.state;
const { spotify, currentUser, apiUrl } = this.props;
const { track: tracks } = playlist;
const hasRightToEdit = this.hasRightToEdit();
Expand Down Expand Up @@ -892,13 +912,16 @@ export default class PlaylistPage extends React.Component<
className="search"
cacheOptions
isClearable
closeMenuOnSelect={false}
loadingMessage={({ inputValue }) =>
`Searching for '${inputValue}'…`
}
loadOptions={this.searchForTrackDebounced}
defaultOptions={cachedSearchResults}
onChange={this.addTrack}
placeholder="Artist followed by track name"
ref={this.addTrackSelectRef}
inputValue={searchInputValue}
onInputChange={this.handleInputChange}
/>
</Card>
)}
Expand Down

0 comments on commit bdbd055

Please sign in to comment.