-
-
Notifications
You must be signed in to change notification settings - Fork 209
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Listening now: allow users to vote on and add tags #2475
Conversation
+ use new TagComponent
always pass an entity MBID, which we need for the add tag component
Actually enable users to write a new tag, and stub implementation of submission mechanism
Yada yada XML yada yada validation yada yada auth token. Auth token is currently missing, will need to wait for LB-1120 to be implemented.
We can't generate the callback frunction from a TagActionType array.map to refactor, the callback functions have to be defined separately, duplicating some code. See https://react.dev/reference/react/useCallback#i-need-to-call-usememo-for-each-list-item-in-a-loop-but-its-not-allowed
1. Use the right auth header (Bearer) 2. refresh token + retry if we get an unauthorized status
Merging is hard when you change the directory structure drastically. In this case we had a types file left over that didn't merge. Applying the necessary changes, and deleting the extraneous file.
I was expecting a format for RG tags (entity MBID at tag. release_group_mbid) like for artist tags, which didn't materialize, so there was no entity MBID available
In the Listening Now page for now, perhaps this should be handled and served from the back-end instead.
Use the pre-defined list of MB genres to select from (searchable select). Updates the react-select component while I'm in there, which isn't used anywhere else as far as i can tell.
I think we can only set items on the global context when we first instantiate it
instead of the object returned by react-select
Using only react-select and some heavy customization + improvements and adaptations to the TagComponent
+ separate into its own file
Preparing to make them reusable
Don't allow to vote + disable input if entity MBID is missing (no matched MBID)
try { | ||
const response = await fetch( | ||
"https://musicbrainz.org/ws/2/genre/all?fmt=txt" | ||
); | ||
const genresList = await response.text(); | ||
globalAppContext.musicbrainzGenres = Array.from(genresList.split("\n")); | ||
} catch (error) { | ||
console.error(error); | ||
} | ||
|
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.
In a follow-up PR, we should probably load those as props from the back-end, and cache them aggressively?
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.
Sounds good to me, Maybe a ticket or ping me tomorrow and I'll add it.
remove useEffect used in TagsComponent
woopsie!
Previous implementation? In any case, method is unused and the /profile/mbtoken doesn't seem to exist anymore.
Only show genre tags, in order of vote count, as well as the user's own tags. We filter out other users' non-genre tags, because it can quickly become too much.
Couldn't see any error messages otherwise, duh.
Don't lose tag rounded corners, + tweak colors a bit.
Mistakes were made !
We need the artist_mbid for example from the original tag
I wonder if we could add support for tagging works here as well? made a Jira ticket as well LB-1326 as a gentle reminder more broadly |
Yes, that has been in the works for a while, but the PR is not quite ready for merging yet, IIRC. |
I stand corrected -- the PR was merged, but due to an unrelated bug, this feature didn't appear in the Metadata Viewer. If you're using the metadata viewer and you can't add tags, log out from MB on your desktop browser and log in again. Then restart the metadata viewer. |
@mayhem i think perhaps @UltimateRiff 's suggestion is that we could also tag the MB Work entity as well as the current entities (Recording, Release/RG, Artist). To answer that, I think it's a good idea, but I'll need to avaluate how to do that clearly. I already want to change the UI of this component to remove the accordion stuff so we can see all the information at once. The only thing the accordions bring is that it is clear(er) which entity you are tagging, so adding a 4th type of entity to tag could become confusing. |
@mayhem I have actually been having that issue with the live version of Listening Now, but not the test version. I'll try that fix~ ...but indeed, I'm talking here about MusicBrainz works entities~ I could definitely see the accordion becoming complicated with works, especially since (unlike releases and recordings), there can be several works attached to the same recording |
This PR takes over from #2072 which had become very much out of date.
This new branch is rebased on fresh master, and implements the missing pieces of this new feature (auth token, refresh and retry mechanism).
User can now vote on and add tags from the front-end !
For now, until we finesse a few details* this is only available on the Listening Now page as a test bed.
Allows users to vote on existing tags
Also allows user to create their own freetext tags:
Select/search for a tag from the MusicBrainz genres :
* < sarcasm > small details like how the tags will look, where we show them, which entities do we want to tag and other trivial points < /sarcasm >
Also left to implement is a way to show the current user's tags (and votes?) when we load or refresh the page.
This will require a lot more API calls I suppose.