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

Isrc links #77

Merged
merged 12 commits into from
Dec 15, 2024
Merged

Isrc links #77

merged 12 commits into from
Dec 15, 2024

Conversation

reubot
Copy link
Contributor

@reubot reubot commented Dec 11, 2024

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

@kellnerd kellnerd added feature New feature or request web Web interface labels Dec 11, 2024
Copy link
Owner

@kellnerd kellnerd left a 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 😅

server/components/ISRC.tsx Outdated Show resolved Hide resolved
server/components/ISRC.tsx Outdated Show resolved Hide resolved
server/components/ISRC.tsx Outdated Show resolved Hide resolved
server/components/SpriteIconWidth.tsx Outdated Show resolved Hide resolved
server/components/ISRC.tsx Outdated Show resolved Hide resolved
reubot and others added 10 commits December 12, 2024 15:42
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>
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
@reubot
Copy link
Contributor Author

reubot commented Dec 12, 2024

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.

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.

Copy link
Owner

@kellnerd kellnerd left a 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.

server/icons/BrandIfpi.tsx Outdated Show resolved Hide resolved
@kellnerd kellnerd changed the base branch from main to dev December 15, 2024 16:21
@kellnerd kellnerd merged commit cd9be8e into kellnerd:dev Dec 15, 2024
2 checks passed
@kellnerd
Copy link
Owner

Just dropping my latest attempt at drawing a Tabler-style IFPI icon here for now:

ifpi-tabler opt

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.
Strangely this rendering issue only happens in context of the Harmony website, the SVG file itself renders just fine in Firefox when I scale it to 70%...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request web Web interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ISRC links on Release Lookup page
2 participants