-
Couldn't load subscription status.
- Fork 287
fix: RTL and Bidirectional Text Issues #11755
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: main
Are you sure you want to change the base?
Conversation
|
Hi everybody |
|
Sorry for the delay. @amirmkia1997 thank you for the PR @hamza221 please give this a test |
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.
Hello I'm trying to review the changes but I have a few questions
Added an RTL/LTR toggle button to the Mail composer toolbar.
I can't see the toggle button on the page or the code can you please share screenshots ?
Are the pre-existing alignment buttons not enough ?
Ensured proper CSS and dir attribute handling so mixed-direction text is displayed and stored in the correct visual and logical order.
I appreciate the css But I'm not convinced it's needed, Maybe I'm not seeing the problem so please if possible attach screenshots from the editor behaving incorrectly. to make the review process easier/
Is the current state of the editor, with automatic direction detection and alignment buttons not enough ?
src/components/TextEditor.vue
Outdated
| } | ||
| this.editorInstance.execute('insertItem', { content, isHtml: this.html }, '!') | ||
| }, | ||
| setupRTLSupport() { |
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.
If you call this at the end of onEditorReady you don't need a timeout
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.
Hi @hamza221,
Thanks for your review! I wanted to clarify a few points regarding the RTL/LTR support in the editor:
RTL/LTR toggle button: There isn’t an actual toggle button added to the toolbar. The RTL/LTR handling is automatic, based on the content typed by the user. I realize the wording may have been misleading — it’s not a manual toggle, it automatically detects the text direction.
Alignment buttons: The existing alignment buttons are still present, but automatic detection ensures mixed-direction text displays correctly without requiring the user to manually change alignment. This improves the user experience when working with mixed English and RTL languages.
CSS changes: The CSS ensures that mixed RTL/LTR text is rendered correctly and stored in the right logical order. I understand it might look unnecessary at first glance, but without it, certain combinations of RTL and LTR text were misaligned. I can attach screenshots showing the incorrect behavior vs. the fixed behavior to make it clearer.
Timeout removal: I’ve moved setupRTLSupport() to the end of onEditorReady as you suggested, so the timeout is no longer needed. This should make the editor initialization cleaner.
Please let me know if you’d like me to provide screenshots of the RTL/LTR rendering before and after these fixes.
Thanks!
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.
Can you please add before/after screenshots of previously problematic text
FYI there is one at #11494 |
Doesn't really show the issue, except the missing buttons. |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
Please fix this bug: Our system supports two languages, and we require an RTL/LTR directional toggle button in the mail editor. |
| mounted() { | ||
| }, |
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.
| mounted() { | |
| }, |
Summary
This pull request addresses issue #11494
, which reports incorrect rendering of bidirectional text (mix of LTR and RTL languages) when composing or replying to messages in the Mail app.
Background
When users type content that mixes English (left-to-right) with Arabic, Persian, or Hebrew (right-to-left), the text direction and punctuation can appear reversed or misaligned. Other mail clients (Outlook, Roundcube, Gmail, etc.) provide an explicit RTL/LTR control to ensure correct layout, but the Nextcloud Mail composer lacked such an option.
Proposed Changes
Added an RTL/LTR toggle button to the Mail composer toolbar.
Ensured proper CSS and dir attribute handling so mixed-direction text is displayed and stored in the correct visual and logical order.
Included fallback styling for environments without explicit direction attributes.
Testing
Verified with multiple combinations of English and Arabic text (including embedded numbers and punctuation) in both new messages and replies.
Confirmed that saved drafts and sent messages retain correct directionality across browsers.
Notes
This change should improve the user experience for anyone composing multilingual emails containing right-to-left scripts.