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

RecentDmConversationsPage: Fix RTL layout problem #338

Merged
merged 1 commit into from
Oct 26, 2023

Conversation

chrisbobbe
Copy link
Collaborator

Fixes: #337

@gnprice
Copy link
Member

gnprice commented Oct 26, 2023

Sure, looks good, merging.

I think at this stage we shouldn't spend any significant amount of time thinking about RTL layout. It won't benefit users until we actually add an RTL locale, and when we do go into that I think it'll be pretty efficient to do so as a general sweep through the app, which we have an issue for (#77). The issue is post-launch because no existing Zulip client supports it, including Zulip web.

I think it does make sense to generally have a habit of taking designs that are given in terms of left and right, and expressing them in our code in terms of start and end, as this PR does. That'll probably reduce the amount we have to change when we eventually do that sweep #77. But it's OK that we may have some places where we say left and right instead, and consequently a mixture where the layout doesn't totally come out correctly because some things are left/right and others are start/end. Those will just be among the things we find when we eventually do #77, and they'll have no user impact until then.

@gnprice gnprice merged commit 2a2cf1b into zulip:main Oct 26, 2023
1 check passed
@chrisbobbe chrisbobbe deleted the pr-recent-dms-rtl-layout branch October 26, 2023 20:43
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.

RecentDmConversationsPage: Breaks with RTL text direction
2 participants