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

Fix misplaced '@' in RTL post meta. #4531

Merged

Conversation

jgarplind
Copy link
Contributor

@jgarplind jgarplind commented Jun 16, 2024

Addresses #4529

The '@' symbol was placed at the right-side of the user handle, but the correct placement is always at the left-side since the user handle is inherently LTR.

Uses techniques from the specification: https://www.unicode.org/reports/tr9/

Detailed problem description

PostMeta.tsx combines two separate text-nodes (handle + display name) into one wrapping text node. The BIDI algorithm (see link above) interprets the combined text as mixed LTR and RTL and applies logic as per the specification. As part of this logic, the @ sign is placed on the right side (i.e. "before" in RTL terms) of the user's handle, since @ is a "neutral character", and the combined text is apparently classified as RTL due to the RTL nature of the initial characters of the text (the display name).

By telling the algorithm to treat the handle (including the @ character) as LTR text, this problem is mitigated.

Show and tell

Before:
image

After:
image

🗒️ Note: Initially, the "After" was still justified towards the profile picture. Upon forking this repo and setting things up again, it was suddenly justified towards the right despite identical code. Now, after rebasing on top of @haileyok's changes, it looks fine again. Hence, I suspect my local environment rather than the code change to have temporarily caused the post meta to be justified to the right.

Testing

✅ Tested on Windows Edge (as shown above).

❌ Don't have the necessary equipment at hand to test on iOS.

❌ Ran into setup problems when trying Android emulator, and didn't spend time trying to set up a Development build.

in other words, it would be helpful to have someone with a fully functioning setup give throw a glance.

The '@' symbol was placed at the right-side of the user handle, but the correct placement is always at the left-side since the user handle is inherently LTR.
@jgarplind jgarplind force-pushed the better-rtl-support-in-post-meta branch from 6395fb0 to a290b2f Compare June 16, 2024 09:46
Copy link
Member

@haileyok haileyok left a comment

Choose a reason for hiding this comment

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

Nice! Thanks for digging at that. This knocks out one of the more annoying problems with LTR, appreciate you looking into it.

@haileyok haileyok merged commit ea7afec into bluesky-social:main Jul 7, 2024
6 checks passed
@jgarplind jgarplind deleted the better-rtl-support-in-post-meta branch July 8, 2024 07:42
estrattonbailey added a commit that referenced this pull request Jul 9, 2024
…cial-proof

* origin/main: (126 commits)
  Update README.md (#4394)
  tweak top padding external (#4755)
  change `contentVisibility` to `contain` (#4752)
  Fix RTL text rendering for display names (#4747)
  Reduce the size of the inner logo in the QR code (#4746)
  Fix misplaced '@' in RTL post meta. (#4531)
  Remove broken and void back button (#4744)
  Ensure `/start` navigates to `/starter-pack` when clicking a link internally (#4745)
  Add missing `to` in StarterPackScreen.tsx string (#4743)
  Video compression in composer (#4638)
  fix slop (#4739)
  Update stats
  Run intl:extract
  Update Japanese translation (#4665)
  Update Korean localization (#4646)
  Update Chinese Localization (#4695)
  Update catalan (#4702)
  Update Indonesian translation (#4706)
  Tweak checkmark size
  Show feedback for Follow button in interstitials (#4738)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants