-
Notifications
You must be signed in to change notification settings - Fork 8
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
Isrc links #77
Isrc links #77
Conversation
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.
Thank you, I like how you tried to keep the ISRC column compact by using icons as links.
Apart from a few nitpicks, I am a bit worried about using the official IFPI logo:
- It is an incomplete version which is hard to recognize, the full version with the "ifpi" text is too detailed for the size used in Harmony.
- It doesn't fit the style of the other stroke-only icons.
- I have no idea about the legal/copyright implications of including the original vectors in this repository.
So what do you think about creating a simple 24x24 grid vector version of their favicon instead? It is just the text IFPI in capital letters which should be easy to design in Inkscape using one of the other custom SVG files as a template.
suggested next step is to seed recordings with matching ISRCs for new releases
I agree that this would be useful, especially for compilations.
It probably isn't as straightforward as this PR, because we have to be careful not to use recordings where the ISRCs are ambiguous and the user should be able to opt out.
Can you create a ticket to discuss this feature in detail?
Depending on the implementation, it might even make the MB links from this PR useless 😅
Please use a JSX fragment here, we don't need the span element in the DOM. Co-authored-by: David Kellner <52860029+kellnerd@users.noreply.github.com>
Co-authored-by: David Kellner <52860029+kellnerd@users.noreply.github.com>
style(web): Update IFPI icon
Update server/components/ISRC.tsx Co-authored-by: David Kellner <52860029+kellnerd@users.noreply.github.com>
…separate internal and external modules. Update server/components/ISRC.tsx Co-authored-by: David Kellner <52860029+kellnerd@users.noreply.github.com>
use butt edges for IFPI logo MusicBrainz icon on the right
Done, I have also squashed it with the the vector I was using before so it the official logo shouldn't be visible in the repo. I have also moved the MB icon to the right and made them size 18. |
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.
LGTM, although I am still not happy that there seems to be no better option for the icon.
I would have loved to have one that matches the rest of the icon set, but my own attempts at it did not look as good as yours.
Just dropping my latest attempt at drawing a Tabler-style IFPI icon here for now: It uses a fixed stroke width of 1px for the four letters, the curve uses the standard stroke of 2px on the 24x24 grid. Unfortunately this seems to be too small for a browser when rendering them at size 18, at least in Firefox the two i-dots are not visible. |
Add links to ISRC search pages for both MB and IFPI (closes #76)
Beethoven_ Symphonies Nos. 7 & 8 - Harmony.html.gz
suggested next step is to seed recordings with matching ISRCs for new releases when MB provider is checked