Skip to content

Conversation

@amirmkia1997
Copy link

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.

@einali
Copy link

einali commented Oct 8, 2025

Hi everybody
is there any update for this pr?

@ChristophWurst
Copy link
Member

ChristophWurst commented Oct 8, 2025

Sorry for the delay.

@amirmkia1997 thank you for the PR

@hamza221 please give this a test

Copy link
Contributor

@hamza221 hamza221 left a 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 ?

}
this.editorInstance.execute('insertItem', { content, isHtml: this.html }, '!')
},
setupRTLSupport() {
Copy link
Contributor

@hamza221 hamza221 Oct 8, 2025

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

Copy link
Author

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!

Copy link
Contributor

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

@ChristophWurst
Copy link
Member

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/

FYI there is one at #11494

@hamza221
Copy link
Contributor

hamza221 commented Oct 8, 2025

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/

FYI there is one at #11494

Doesn't really show the issue, except the missing buttons.

@github-actions
Copy link

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

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.)

@nikbinmarjan
Copy link

Please fix this bug: Our system supports two languages, and we require an RTL/LTR directional toggle button in the mail editor.

Comment on lines +205 to +206
mounted() {
},
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
mounted() {
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants