Skip to content
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

LB-613: Show artist name on the release graph #903

Merged
merged 6 commits into from Jun 10, 2020

Conversation

ishaanshah
Copy link
Collaborator

@ishaanshah ishaanshah commented Jun 5, 2020

Problem

We should show the artist name on the release graph. More than one artists can have releases with the same name and it can be hard to guess which artist the release belongs to.

Solution

This PR adds artist names to the release graphs. In addition to that links pointing to the particular entity on MusicBrainz are also added if the MBID is present.

Action

Should be merged after #900

2020-06-05-190825_1918x932_scrot

@ishaanshah ishaanshah changed the title [WIP]LB-613: Show artist name on the release graph LB-613: Show artist name on the release graph Jun 5, 2020
@ishaanshah ishaanshah marked this pull request as draft June 5, 2020 13:46
@ishaanshah ishaanshah force-pushed the ux_improv branch 2 times, most recently from 57f0a91 to 6963084 Compare June 8, 2020 02:17
@ishaanshah ishaanshah changed the title LB-613: Show artist name on the release graph [WIP] LB-613: Show artist name on the release graph Jun 9, 2020
@ishaanshah ishaanshah marked this pull request as ready for review June 10, 2020 03:02
@ishaanshah ishaanshah changed the title [WIP] LB-613: Show artist name on the release graph LB-613: Show artist name on the release graph Jun 10, 2020
Copy link
Collaborator

@paramsingh paramsingh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks reasonable to me, can you post a screenshot of how it looks on mobile?

@ishaanshah
Copy link
Collaborator Author

Looks reasonable to me, can you post a screenshot of how it looks on mobile?

2020-06-10-154828_290x574_scrot

@@ -54,7 +54,7 @@ const searchForSpotifyTrack = async (
return null;
};

const getArtistLink = (listen: any) => {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

woohoo!

@@ -73,13 +73,12 @@ const getArtistLink = (listen: any) => {
return artistName;
};

// TODO: remove this "any" when a listen type has been defined.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔥

@paramsingh paramsingh merged commit c629815 into metabrainz:master Jun 10, 2020
@ishaanshah ishaanshah deleted the ux_improv branch June 12, 2020 04:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants