Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Fix alignment of RTL messages #12837

Merged
merged 17 commits into from
Jul 31, 2024
Merged

Fix alignment of RTL messages #12837

merged 17 commits into from
Jul 31, 2024

Conversation

dbkr
Copy link
Member

@dbkr dbkr commented Jul 29, 2024

Screenshot 2024-07-29 at 17 46 50

Inspired by #5453 but hopefully with the edited marker in the right place.

This is now passing type checking now (albeit not hugely elegantly).

One kicker is that I've had to fix emote bodies. I've done this by wrapping them in the same way. This does mean that if you have an rtl display name, all your emotes will be right-justified, even if you're emoting in an ltr language, like so:

Screenshot 2024-07-31 at 12 54 26

...this is a bit weird perhaps? But possibly the least weird of all options.

Checklist

  • Tests written for new code (and old code if feasible).
  • New or updated public/exported symbols have accurate TSDoc documentation.
  • Linter and other CI checks pass.
  • Sign-off given on the changes (see CONTRIBUTING.md).

Inspired by #5453 but
hopefully with the edited marker in the right place.

This is a PoC: types aren't correct and the style needs pulling
out to a class. Plus it would probably need more visual tests added.
If this looks acceptable, I can make these changes.
@ahangarha
Copy link
Contributor

Good move.

But it seems there are some spacing issues
image

@dbkr
Copy link
Member Author

dbkr commented Jul 30, 2024

I think the right hand side one was just an erroneous space character in the actual text. The one between the text and the edited marker should hopefully be simple enough to fix.

@dbkr dbkr added the T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems label Jul 31, 2024
This will cause them always be right-justified if the display name
is rtl.
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

Looks sane

@dbkr dbkr added this pull request to the merge queue Jul 31, 2024
Merged via the queue into develop with commit a0c029c Jul 31, 2024
29 checks passed
@dbkr dbkr deleted the dbkr/message-body-text-direction branch July 31, 2024 22:39
@SPiRiT369
Copy link
Contributor

As RTL user, rtl support is very useful for me. But in my opinion, this change makes the chat experience worse and not aesthetic.
If the browser window is large or maximized and you have a flow of messages, this makes it unclear who wrote what, and your eyes need to jump to the left to see who wrote the messages.
IMHO without aligning the avatars+names to the right as well, this change is irrelevant and does not serve the purpose.
The proper way to do that (IMHO) is to keep the previous style (rtl text aligned to the left). The decision about aligning messages to the right should be based on the UI language (and if that's a problem then by a system setting). And when in RTL mode, the WHOLE conversation (including LTR messages, avatars and names) should align to the right as well.

Excuse my poor gimp abilities, it's just to demonstrate how more aesthetic it looks having the messages on the same side with the avatars+names, even when ltr/rtl text is not adjusted correctly.

image

@ahangarha
Copy link
Contributor

I think there are two things to be considered separately:

  • The overall layout should be determined by the chosen language for the UI.
  • Individual content (messages) should render in correct direction regardless of the overall layout.

Moreover, we cannot judge based on a single message look.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
T-Defect Bugs, crashes, hangs, vulnerabilities, or other reported problems
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants