-
Notifications
You must be signed in to change notification settings - Fork 22
feat: Apple Music, Amazon Music, and Qobuz services #147
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
base: master
Are you sure you want to change the base?
Conversation
added support for Apple Music: album, artist, playlist, and track
added support for Amazon Music
Added support for Qobuz album and track
…fixed AM album ID regex; fixed Qobuz regex
octfx
left a comment
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 very much for this PR!
Overall looks very good, two things:
- Do you have information on the
zoneparameter used by Qobuz? CanUS-enbe used as a "default" value worldwide? - Regarding the added corner radii, I am currently undecided on if they should be kept. Omitting lets users style the embed, keeping them honors the brand style of the embed
I am currently leaning towards omitting the added border radius, but I am open for discussion.
If we keep it, then we should at least change from px to rem 🙂
I will try to see if I can make a Qobuz account for an different region to see how the zone and track IDs change and how that affects it. For the radii, I believe it would be better to default to having the rounded corners due to an iframe/prefers-color-scheme mistmatch quirk in all modern browsers. This occurs for MediaWiki skins that use the CSS prefers-color-scheme for setting light/dark theming (this is used for matching the website to the OS or browser color theme settings). These service providers did not set their color scheme meta attribute in the header: In the case of the music embeds, when the theme is dark, the browser adds a white background, which makes the corners appear like this:
The service providers just set a border radius on the default embed code you get from them. I think it's to cover compatibility with older browsers that do not support the color-scheme CSS property? Though they really should have added the meta tag anyway. There also isn't a way to remove the forced background color with CSS. |
I had just grabbed from their default embed code which used px. Looking deeper, I see that Apple, Amazon, and Deezer use px in the inner HTML radius, but Spotify mismatched and used rem on the inside. 1rem doesn't always equal 16px if the browser's default font size is changed, so a rem border radius would change with the font size. I think we should definitely update Spotify's to rem so it matches the inner styling, but maybe keep px for the others so that if the font size is being increased, the user doesn't see the embed slowly turn into a perfect circle which could eventually start cutting off content. It's definitely an edge case, and I am not aware of how often people change their browser's default font size. I also wonder if on iOS and Android, do browsers use the OS font size as their default? I think more people change that one. |
|
@nicholasbrantley Just to accelerate the review a bit, would you be able to share the wikitext you used for the test page? |
Sure thing, below is examples for the affected services. You can also view them implemented on this wiki page. |



This is for #144, #145, and #146. It adds these embeds:
Additional fixes:
Some notes: