From bdbd0558b8091cd98ee12d24fe95100d5e3c69a1 Mon Sep 17 00:00:00 2001 From: Monkey Do Date: Wed, 10 Feb 2021 12:00:16 +0100 Subject: [PATCH] LB-778: Don't clear add-to-playlist search input on blur (#1254) * 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 --- .../js/src/__mocks__/playlistPageProps.json | 39 ++++++++++ .../static/js/src/playlists/Playlist.test.tsx | 73 +++++++++++++++++++ .../static/js/src/playlists/Playlist.tsx | 45 +++++++++--- .../__snapshots__/Playlist.test.tsx.snap | 3 + package.json | 12 +-- 5 files changed, 155 insertions(+), 17 deletions(-) create mode 100644 listenbrainz/webserver/static/js/src/__mocks__/playlistPageProps.json create mode 100644 listenbrainz/webserver/static/js/src/playlists/Playlist.test.tsx create mode 100644 listenbrainz/webserver/static/js/src/playlists/__snapshots__/Playlist.test.tsx.snap diff --git a/listenbrainz/webserver/static/js/src/__mocks__/playlistPageProps.json b/listenbrainz/webserver/static/js/src/__mocks__/playlistPageProps.json new file mode 100644 index 0000000000..daf90b0301 --- /dev/null +++ b/listenbrainz/webserver/static/js/src/__mocks__/playlistPageProps.json @@ -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 80s 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" +} diff --git a/listenbrainz/webserver/static/js/src/playlists/Playlist.test.tsx b/listenbrainz/webserver/static/js/src/playlists/Playlist.test.tsx new file mode 100644 index 0000000000..e25ec49aa4 --- /dev/null +++ b/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(); + expect(wrapper.html()).toMatchSnapshot(); + }); + it("does not clear the add-a-track input on blur", async () => { + const wrapper = shallow(); + 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"); + }); +}); diff --git a/listenbrainz/webserver/static/js/src/playlists/Playlist.tsx b/listenbrainz/webserver/static/js/src/playlists/Playlist.tsx index 117bd9c04f..cdf353dda4 100644 --- a/listenbrainz/webserver/static/js/src/playlists/Playlist.tsx +++ b/listenbrainz/webserver/static/js/src/playlists/Playlist.tsx @@ -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, @@ -55,6 +55,8 @@ export interface PlaylistPageState { playlist: JSPFPlaylist; recordingFeedbackMap: RecordingFeedbackMap; loading: boolean; + searchInputValue: string; + cachedSearchResults: OptionType[]; } type OptionType = { label: string; value: ACRMSearchResult }; @@ -76,7 +78,6 @@ export default class PlaylistPage extends React.Component< private spotifyPlaylist?: SpotifyPlaylistObject; private searchForTrackDebounced: any; private brainzPlayer = React.createRef(); - private addTrackSelectRef = React.createRef>(); private socket!: SocketIOClient.Socket; @@ -96,6 +97,8 @@ export default class PlaylistPage extends React.Component< playlist: props.playlist?.playlist || {}, recordingFeedbackMap: {}, loading: false, + searchInputValue: "", + cachedSearchResults: [], }; this.APIService = new APIService( @@ -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([ @@ -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 => { @@ -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); @@ -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(); @@ -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} /> )} diff --git a/listenbrainz/webserver/static/js/src/playlists/__snapshots__/Playlist.test.tsx.snap b/listenbrainz/webserver/static/js/src/playlists/__snapshots__/Playlist.test.tsx.snap new file mode 100644 index 0000000000..20cc5f9439 --- /dev/null +++ b/listenbrainz/webserver/static/js/src/playlists/__snapshots__/Playlist.test.tsx.snap @@ -0,0 +1,3 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`PlaylistPage renders correctly 1`] = `"

1980s flashback jams
Public playlist by iliekcomputers

1 tracks
Created: 1/4/2021, 2:43:58 PM
Last modified: 1/18/2021, 4:44:22 PM
Your lame 80s music
Drag to reorder
Endless Boogie
added 1 day ago
by troi-bot
  Add a track
Artist followed by track name

Edit playlist

Add collaborator

Delete playlist

You are about to delete playlist 1980s flashback jams.
This action cannot be undone.
No album art
"`; diff --git a/package.json b/package.json index c91512a3c9..38bc95f420 100644 --- a/package.json +++ b/package.json @@ -13,12 +13,12 @@ "build": "npm run build:dev", "format": "eslint ./static/js/src --ext .js,jsx,ts,tsx --fix --quiet", "format:ci": "eslint ./static/js/src --ext .js,jsx,ts,tsx -o eslint.xml --format checkstyle", - "test": "jest", - "test:ci": "jest --ci --reporters=default --reporters=jest-junit", - "test:watch": "jest --watch", - "test:update-snapshots": "jest -u", - "test:coverage": "jest --coverage --colors", - "test:debug": "node --inspect node_modules/.bin/jest --runInBand", + "test": "TZ='Europe/London' jest", + "test:ci": "TZ='Europe/London' jest --ci --reporters=default --reporters=jest-junit", + "test:watch": "TZ='Europe/London' jest --watch", + "test:update-snapshots": "TZ='Europe/London' jest -u", + "test:coverage": "TZ='Europe/London' jest --coverage --colors", + "test:debug": "TZ='Europe/London' node --inspect node_modules/.bin/jest --runInBand", "type-check": "tsc --noEmit" }, "author": "MetaBrainz Foundation Inc.",