LB-1759: Added a checkbox to make multiple listens or albums keeping the modal open #3269
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -101,12 +101,17 @@ export default NiceModal.create(() => { | |
| const [customTimestamp, setCustomTimestamp] = useState(false); | ||
| const [selectedDate, setSelectedDate] = useState(new Date()); | ||
| const [invertOrder, setInvertOrder] = useState(false); | ||
| const [keepModalOpen, setKeepModalOpen] = useState(false); | ||
| // Used for the automatic switching and search trigger if pasting URL for another entity type | ||
| const [textToSearch, setTextToSearch] = useState<string>(); | ||
|
|
||
| const closeModal = useCallback(() => { | ||
| modal.hide(); | ||
| document?.body?.classList?.remove("modal-open"); | ||
| const backdrop = document?.querySelector(".modal-backdrop"); | ||
| if (backdrop) { | ||
| backdrop.remove(); | ||
| } | ||
| setTimeout(modal.remove, 200); | ||
| }, [modal]); | ||
|
|
||
|
|
@@ -186,7 +191,14 @@ export default NiceModal.create(() => { | |
| />, | ||
| { toastId: "added-listens-success" } | ||
| ); | ||
|
|
||
| if (!keepModalOpen) { | ||
| closeModal(); | ||
| } else { | ||
| // Reset the form state but keep the modal open | ||
| setSelectedListens([]); | ||
| setTextToSearch(""); | ||
| } | ||
| } catch (error) { | ||
| handleError( | ||
| error, | ||
|
|
@@ -211,6 +223,7 @@ export default NiceModal.create(() => { | |
| APIService, | ||
| closeModal, | ||
| handleError, | ||
| keepModalOpen, | ||
| ]); | ||
|
|
||
| const switchMode = React.useCallback( | ||
|
|
@@ -369,19 +382,31 @@ export default NiceModal.create(() => { | |
| </div> | ||
| </div> | ||
| </div> | ||
| <div | ||
| className="modal-footer" | ||
| style={{ display: "flex", alignItems: "center", gap: "1rem" }} | ||
| > | ||
| <div style={{ flex: 1 }}> | ||
| <label style={{ userSelect: "none", cursor: "pointer" }}> | ||
| <input | ||
| type="checkbox" | ||
| checked={keepModalOpen} | ||
| onChange={(e) => setKeepModalOpen(e.target.checked)} | ||
| style={{ marginRight: "0.5em" }} | ||
| /> | ||
| Add another | ||
| </label> | ||
| </div> | ||
| <button | ||
| type="button" | ||
| className="btn btn-default" | ||
| onClick={closeModal} | ||
| > | ||
| Close | ||
| </button> | ||
| <button | ||
| type="button" | ||
|
There was a problem hiding this comment. Is this change required? Just out of curiosity. There was a problem hiding this comment. Yes, this change is needed. I changed it from There was a problem hiding this comment. I see! The way to ensure the page is not reloaded is to add use the event's listenbrainz-server/frontend/js/src/components/Navbar.tsx Lines 25 to 26 in 03d06f4 Generally, keeping the type:submit is important for accessibility (for example for screen readers for visually impaired users). There was a problem hiding this comment. Hi @MonkeyDo , I just wanted to confirm a few things about my
See my changed import React, { useCallback, useContext, useState } from "react";
import DateTimePicker from "react-datetime-picker/dist/entry.nostyle";
import { FontAwesomeIcon } from "@fortawesome/react-fontawesome";
import { IconProp } from "@fortawesome/fontawesome-svg-core";
import { faCalendar } from "@fortawesome/free-regular-svg-icons";
import NiceModal, { useModal } from "@ebay/nice-modal-react";
import { toast } from "react-toastify";
import { Link } from "react-router-dom";
import { add } from "date-fns";
import { get } from "lodash";
import GlobalAppContext from "../../utils/GlobalAppContext";
import { convertDateToUnixTimestamp } from "../../utils/utils";
import { ToastMsg } from "../../notifications/Notifications";
import AddSingleListen from "./AddSingleListen";
import AddAlbumListens from "./AddAlbumListens";
import Pill from "../../components/Pill";
enum SubmitListenType {
"track",
"album",
}
export type MBTrackWithAC = MusicBrainzTrack & WithArtistCredits;
export function getListenFromRecording(
recording: MusicBrainzRecordingWithReleasesAndRGs,
date?: Date,
release?: MusicBrainzRelease & WithReleaseGroup
): Listen {
const releaseOrCanonical = release ?? recording.releases[0];
const listen: Listen = {
listened_at: date ? convertDateToUnixTimestamp(date) : 0,
track_metadata: {
artist_name:
recording["artist-credit"]
?.map((artist) => `${artist.name}${artist.joinphrase}`)
.join("") ?? "",
track_name: recording.title,
release_name: releaseOrCanonical?.title,
additional_info: {
release_mbid: releaseOrCanonical?.id,
release_group_mbid: releaseOrCanonical?.["release-group"]?.id,
recording_mbid: recording.id,
submission_client: "listenbrainz web",
artist_mbids: recording["artist-credit"].map((ac) => ac.artist.id),
},
},
};
if (recording.length) {
// Cannot send a `null` duration
listen.track_metadata.additional_info!.duration_ms = recording.length;
}
return listen;
}
export const getListenFromTrack = (
track: MusicBrainzTrack & WithArtistCredits,
date: Date,
release?: MusicBrainzRelease
): Listen => {
const listen: Listen = {
listened_at: convertDateToUnixTimestamp(date),
track_metadata: {
track_name: track.title,
artist_name:
track["artist-credit"]
?.map((artist) => `${artist.name}${artist.joinphrase}`)
.join("") ?? "",
additional_info: {
recording_mbid: track.recording.id,
submission_client: "listenbrainz web",
track_mbid: track.id,
tracknumber: track.number,
artist_mbids: track["artist-credit"].map((ac) => ac.artist.id),
},
},
};
if (track.length) {
// Cannot send a `null` duration
listen.track_metadata.additional_info!.duration_ms = track.length;
} else if (track.recording.length) {
// Cannot send a `null` duration
listen.track_metadata.additional_info!.duration_ms = track.recording.length;
}
if (release) {
listen.track_metadata.additional_info!.release_mbid = release.id;
listen.track_metadata.release_name = release.title;
}
return listen;
};
// Use a default of 1 minute for the track length if not available in metadata
export const DEFAULT_TRACK_LENGTH_SECONDS = 60;
export default NiceModal.create(() => {
const modal = useModal();
const { APIService, currentUser } = useContext(GlobalAppContext);
const { auth_token } = currentUser;
const [listenOption, setListenOption] = useState<SubmitListenType>(
SubmitListenType.track
);
const [selectedListens, setSelectedListens] = useState<Listen[]>([]);
const [customTimestamp, setCustomTimestamp] = useState(false);
const [selectedDate, setSelectedDate] = useState(new Date());
const [invertOrder, setInvertOrder] = useState(false);
const [keepModalOpen, setKeepModalOpen] = useState(false);
// Used for the automatic switching and search trigger if pasting URL for another entity type
const [textToSearch, setTextToSearch] = useState<string>();
const closeModal = useCallback(() => {
modal.hide();
document?.body?.classList?.remove("modal-open");
const backdrop = document?.querySelector(".modal-backdrop");
if (backdrop) {
backdrop.remove();
}
setTimeout(modal.remove, 200);
}, [modal]);
const handleError = useCallback(
(error: string | Error, title?: string): void => {
if (!error) {
return;
}
toast.error(
<ToastMsg
title={title || "Error"}
message={typeof error === "object" ? error.message : error.toString()}
/>,
{ toastId: "add-listen-error" }
);
},
[]
);
const submitListens = useCallback(async () => {
if (auth_token) {
let payload: Listen[] = [];
const listenType: ListenType =
selectedListens?.length <= 1 ? "single" : "import";
// Use the user-selected date, default to "now"
const date = customTimestamp ? selectedDate : new Date();
if (selectedListens?.length) {
let orderedSelectedListens = [...selectedListens];
if (invertOrder) {
orderedSelectedListens = orderedSelectedListens.reverse();
}
let cumulativeDateTime = date;
payload = orderedSelectedListens.map((listen) => {
// Now we need to set the listening time for each listen
const modifiedListen = { ...listen };
const durationMS = get(
modifiedListen,
"track_metadata.additional_info.duration_ms"
);
const timeToAdd =
(durationMS && durationMS / 1000) ??
modifiedListen.track_metadata.additional_info?.duration ??
DEFAULT_TRACK_LENGTH_SECONDS;
// We either use the previous value of cumulativeDateTime,
// or if inverting listening order directly use the new value newTime
const newTime = add(cumulativeDateTime, {
seconds: invertOrder ? -timeToAdd : timeToAdd,
});
if (invertOrder) {
modifiedListen.listened_at = convertDateToUnixTimestamp(newTime);
} else {
modifiedListen.listened_at = convertDateToUnixTimestamp(
cumulativeDateTime
);
}
// Then we assign the new time value for the next iteration
cumulativeDateTime = newTime;
return modifiedListen;
});
}
if (!payload?.length) {
return;
}
const listenOrListens = payload.length > 1 ? "listens" : "listen";
try {
const response = await APIService.submitListens(
auth_token,
listenType,
payload
);
await APIService.checkStatus(response);
toast.success(
<ToastMsg
title="Success"
message={`You added ${payload.length} ${listenOrListens}`}
/>,
{ toastId: "added-listens-success" }
);
if (!keepModalOpen) {
closeModal();
} else {
// Reset the form state but keep the modal open
setSelectedListens([]);
setTextToSearch("");
}
} catch (error) {
handleError(
error,
`Error while submitting ${payload.length} ${listenOrListens}`
);
}
} else {
toast.error(
<ToastMsg
title="You need to be logged in to submit listens"
message={<Link to="/login/">Log in here</Link>}
/>,
{ toastId: "auth-error" }
);
}
}, [
auth_token,
selectedListens,
customTimestamp,
selectedDate,
invertOrder,
APIService,
closeModal,
handleError,
keepModalOpen,
]);
const switchMode = React.useCallback(
(pastedURL: string) => {
if (listenOption === SubmitListenType.track) {
setListenOption(SubmitListenType.album);
} else if (listenOption === SubmitListenType.album) {
setListenOption(SubmitListenType.track);
}
setTimeout(() => {
// Trigger search in the inner (grandchild) search input component by modifying the textToSearch prop in child component
// Give it some time to allow re-render and trigger search in the correct child component
setTextToSearch(pastedURL);
}, 200);
setTimeout(() => {
// Reset text trigger
setTextToSearch("");
}, 500);
},
[listenOption]
);
const userLocale = navigator.languages?.length
? navigator.languages[0]
: navigator.language;
const handleFormSubmit = (e: React.FormEvent<HTMLFormElement>) => {
e.preventDefault();
submitListens();
};
return (
<div
className={`modal fade ${modal.visible ? "in" : ""}`}
id="AddListenModal"
tabIndex={-1}
role="dialog"
aria-labelledby="AddListenModalLabel"
data-backdrop="static"
>
<div className="modal-dialog" role="document">
<form className="modal-content" onSubmit={handleFormSubmit}>
<div className="modal-header">
<button
type="button"
className="close"
data-dismiss="modal"
aria-label="Close"
onClick={closeModal}
>
<span aria-hidden="true">×</span>
</button>
<h4 className="modal-title" id="AddListenModalLabel">
Add Listens
</h4>
</div>
<div className="modal-body">
<div className="add-listen-header">
<Pill
active={listenOption === SubmitListenType.track}
onClick={() => {
setListenOption(SubmitListenType.track);
}}
type="secondary"
>
Add track
</Pill>
<Pill
active={listenOption === SubmitListenType.album}
onClick={() => {
setListenOption(SubmitListenType.album);
}}
type="secondary"
>
Add album
</Pill>
</div>
{listenOption === SubmitListenType.track && (
<AddSingleListen
onPayloadChange={setSelectedListens}
switchMode={switchMode}
initialText={textToSearch}
/>
)}
{listenOption === SubmitListenType.album && (
<AddAlbumListens
onPayloadChange={setSelectedListens}
switchMode={switchMode}
initialText={textToSearch}
/>
)}
<hr />
<div className="timestamp">
<h5>Timestamp</h5>
<div className="timestamp-entities">
<Pill
active={customTimestamp === false}
onClick={() => {
setCustomTimestamp(false);
setSelectedDate(new Date());
}}
type="secondary"
>
Now
</Pill>
<Pill
active={customTimestamp === true}
onClick={() => {
setCustomTimestamp(true);
setSelectedDate(new Date());
}}
type="secondary"
>
Custom
</Pill>
<div className="timestamp-date-picker">
<div>
<label htmlFor="starts-at">
<input
name="invert-timestamp"
type="radio"
checked={invertOrder === false}
id="starts-at"
aria-label="Set the time of the beginning of the album"
onChange={() => {
setInvertOrder(false);
}}
/>
Starts at:
</label>
<label htmlFor="ends-at">
<input
name="invert-timestamp"
type="radio"
checked={invertOrder === true}
id="ends-at"
aria-label="Set the time of the end of the album"
onChange={() => {
setInvertOrder(true);
}}
/>
Finishes at:
</label>
</div>
<DateTimePicker
value={selectedDate}
onChange={(newDateTimePickerValue: Date) => {
setSelectedDate(newDateTimePickerValue);
}}
calendarIcon={
<FontAwesomeIcon icon={faCalendar as IconProp} />
}
maxDate={new Date()}
clearIcon={null}
format={userLocale ? undefined : "yyyy-MM-dd hh:mm:ss"}
locale={userLocale}
maxDetail="second"
disabled={!customTimestamp}
/>
</div>
</div>
</div>
</div>
<div
className="modal-footer"
style={{ display: "flex", alignItems: "center", gap: "1rem" }}
>
<div style={{ flex: 1 }}>
<label style={{ userSelect: "none", cursor: "pointer" }}>
<input
type="checkbox"
checked={keepModalOpen}
onChange={(e) => setKeepModalOpen(e.target.checked)}
style={{ marginRight: "0.5em" }}
/>
Add another
</label>
</div>
<button
type="button"
className="btn btn-default"
onClick={closeModal}
>
Close
</button>
<button
type="submit"
className="btn btn-success"
disabled={!selectedListens?.length}
>
Submit {selectedListens.length ?? 0} listen
{selectedListens.length > 1 ? "s" : ""}
</button>
</div>
</form>
</div>
</div>
);
});
But I ran into a small issue with browser validation: When I select tracks and try to submit, the browser still asks for the input field to be filled in (because it’s marked as One More thing it is running fine with Add Album see below : To fix that, I made the input field required only if no tracks are selected. In requiredInput={selectedRecordings.length === 0}And in <input required={props.requiredInput} />After that, TypeScript gave me an error saying:
I Think this is happening because now all places where I use So now I’m wondering:
Thank You for the help !! There was a problem hiding this comment. Nice work :) For the form submission, you don't need a new handleFormSubmit function, you can directly pass the submitListens function like so: This was exactly correct, no need to change it: Then modify the All I did was, like you, define the expected event type, and add the prevenDefault call right before the rest of the function. Regarding the required input, looks like I didn't think everything through ;) listenbrainz-server/frontend/js/src/utils/SearchTrackOrMBID.tsx Lines 43 to 55 in 7f44897 |
||
| className="btn btn-success" | ||
| disabled={!selectedListens?.length} | ||
| onClick={submitListens} | ||
| > | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have the impression that this is not working as expected?
At least in the video in the PR description it shows two selected listens one under the other, when I would expect clicking the 'submit' button to clear the current selection
I think this is possibly because some of the logic is in the child components AddSingleListen and AddAlbumListens ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MonkeyDo Thanks for catching this ! You are right the selected listens are not being fully cleared after submitting, which is why multiple listens are showing up in the modal. I think this is because, even though I’m resetting the
selectedListensstate in the parent, the child componentsAddSingleListenandAddAlbumListensmight not be resetting their own state or syncing properly.I’ll dig into how the child components manage their state and make sure everything resets as expected when “Add another” is checked. If you have any suggestions for the best way to reset the child components, I would love to hear them! Otherwise, I will update the PR soon with a fix. Thanks again for your helpful feedback
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At a quick glance I didn't find a good way to clear the child components' state.
The two ideas I have in mind:
keyprop like we do here for example. I'm not sure that'll work or be efficient, but it might be worth a try?searchInputRef, defined here in the child withforwardRefand useImperativeHandle, assigned here in the parent and used to trigger an internal methodtriggerSearchhere:listenbrainz-server/frontend/js/src/user/components/AddAlbumListens.tsx
Lines 110 to 119 in 03d06f4
Also see the react docs page for context: https://react.dev/reference/react/useImperativeHandle
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Forgot to paste this link about the
keyprop: https://react.dev/reference/react/useState#resetting-state-with-a-keyUh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hey @MonkeyDo I tried the first option to reset the form state by passing a key prop for child components
AddSingleListenandAddAlbumListensand it is not working !! , so now I will go ahead with the second option !!!There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@MonkeyDo So After proceeding with the second option and doing required changes to related files this is the conclusion you can see in the video below :
Your.Listens.-.ListenBrainz.and.10.more.pages.-.Personal.-.Microsoft_.Edge.2025-05-16.04-19-33.online-video-cutter.com.mp4
So can I go ahead to merge the commit for this pr ?
And thankyou for your efforts you put in this to guide me step by step I really appreciate it !!
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes you can push the commit, I'll do another review.
nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For sure !!