Skip to content

fix: render entity links correctly with RTL text - #233

Merged
kellnerd merged 3 commits into
kellnerd:mainfrom
arsinclair:fix/rtl-text
Aug 30, 2026
Merged

fix: render entity links correctly with RTL text#233
kellnerd merged 3 commits into
kellnerd:mainfrom
arsinclair:fix/rtl-text

Conversation

@arsinclair

@arsinclair arsinclair commented Jun 24, 2026

Copy link
Copy Markdown
Contributor

Fixes #229.

Entity links containing right-to-left text could be visually reordered by the browser, making it unclear which provider icons and entity names belonged to each link.

This change:

  • renders all entity-link groups as inline flex containers, covering artists, labels, recordings, and other linked entities;
  • wraps alternative-value contents in a dedicated flex container;
  • keeps the alternative values themselves as list items, preserving their bullet points;
  • removes the artist- and release-specific flex workarounds (rolls back changes in the first commit, but didn't wanna squash for posterity).

Before the change:
image

After the change:
image

Test link:

https://harmony.pulsewidth.org.uk/release?url=https%3A%2F%2Ftidal.com%2Falbum%2F529736650&gtin=&region=GB%2CUS%2CDE%2CJP&musicbrainz=&deezer=&itunes=&spotify=&tidal=&qobuz=&beatport=

@kellnerd kellnerd added bug Something isn't working web Web interface labels Jun 24, 2026

@kellnerd kellnerd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Very interesting, I am not even sure if the previous behavior is some kind of bug in the rendering engine... thank you for reporting @YoGo9!

You are right @arsinclair, in order to keep the entity name as link without this weird glitch, flex-boxes seem to be the proper solution as far as I can see.
However, the list items of the alternative values can't be flex-boxes themselves, with this change they lose their bullet points. We probably need an additional container to achieve the desired outcome.
Also, the same issue potentially affects all entity links, so the CSS changes should target those and not just artist credits.

@arsinclair

arsinclair commented Jun 25, 2026

Copy link
Copy Markdown
Contributor Author

Awesome, then I hope maybe @YoGo9 or @kellnerd can take it forward from here? My PR was just a scratchpad example to show what I mean, neither did I test it well nor have I verified this issue in other areas of the app :) I'd be glad to complete the implementation, but unfortunately I'm very short on time right now.

@YoGo9

YoGo9 commented Jul 7, 2026

Copy link
Copy Markdown

(I tried, and wasn't successful)

@arsinclair

Copy link
Copy Markdown
Contributor Author

Which part wasn't successful? The solution doesn't work or you were not able to adapt it to Kellner's comments?

@YoGo9

YoGo9 commented Jul 7, 2026

Copy link
Copy Markdown

Wasn't able to adapt to kellnerds comment

@arsinclair

Copy link
Copy Markdown
Contributor Author

Okay, I'll take a look

@arsinclair arsinclair changed the title fix: convert artist credit rows to flexboxes to handle rtl correctly fix: render entity links correctly with RTL text Aug 30, 2026
@arsinclair
arsinclair requested a review from kellnerd August 30, 2026 05:26
@arsinclair

Copy link
Copy Markdown
Contributor Author

Done I think, testing locally and everything seems to render correctly.

@kellnerd kellnerd left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for coming back to this. Looks good to me as far as I can judge Hebrew, nice and simple CSS changes.

@kellnerd
kellnerd merged commit be77d74 into kellnerd:main Aug 30, 2026
4 checks passed
@arsinclair
arsinclair deleted the fix/rtl-text branch August 30, 2026 19:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working web Web interface

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants