Make SMS messages content UI RTL-friendly

RESOLVED FIXED in 2.2 S1 (5dec)

Status

Firefox OS
Gaia::SMS
RESOLVED FIXED
4 years ago
3 years ago

People

(Reporter: Nefzaoui, Assigned: steveck)

Tracking

unspecified
2.2 S1 (5dec)
All
Gonk (Firefox OS)
Dependency tree / graph

Firefox Tracking Flags

(ux-b2g:2.2)

Details

(Whiteboard: [p=2 for 2.1S9][p=1 for 2.2S1])

Attachments

(5 attachments)

Comment hidden (empty)
(Reporter)

Updated

4 years ago
Blocks: 906270, 964033

Updated

4 years ago
Blocks: 1064569
No longer blocks: 906270
ux-b2g: --- → 2.2
I think we'll need to use either <bdi> or `dir="auto"` attribute, or the CSS property "unicode-bidi: -moz-isolate/isolate-override/plaintext", I'm not sure which one yet. I think we'll need to play with real arabic or hebrew messages to understand what we need to use.
We need to check also RTL message in a LTR document, and LTR message in RTL document, and even RTL messages containing LTR content and LTR messages containing RTL content. Lots of cases to check (would be easier with data in the desktop mock).

We need to handle the bubble position only if the spec is clear. If it's not let's do this in a separate bug.
Blocks: 1096317
Whiteboard: [p=2]
Target Milestone: --- → 2.1 S9 (21Nov)
(Assignee)

Updated

3 years ago
Assignee: nobody → schung
(Assignee)

Comment 3

3 years ago
Created attachment 8524519 [details] [review]
Link to github

Hi Julien, based on bidi meeting last week we will need to mirror the bubble layout, so I adjust the bubble and attachment layout for RTL. For the text content, I use unicode-bidi: -moz-plaintext to force the text always display the correct direction on matter user choose which language. Using dir="auto" might lead similar result, but there is still a major difference while bubble contains both ltr and rtl language. dir="auto" will only apply 1 direction for entire content, but -moz-plaintext could apply different direction depends on the text, which I think would be more appropriate.

Hi Ahmed, could you please help verify if this patch works as you expected? Thanks!
Attachment #8524519 - Flags: feedback?(nefzaoui.ahmed)
Attachment #8524519 - Flags: feedback?(felash)
Comment on attachment 8524519 [details] [review]
Link to github

I tried various other values instead of "-moz-plaintext" and I also tried 'dir="auto"' but in the end it's what you've done that looks better.

Let's see what Ahmed will say :) Ahmed, can you try these cases:
* messages that have both RTL and LTR content
* a message that starts with arabic, and another that starts with latin text (but still contains both type of content)
* using a phone configured in arabic, and a phone configured in english

Thanks!
Attachment #8524519 - Flags: feedback?(felash) → feedback+
(Assignee)

Comment 5

3 years ago
Comment on attachment 8524519 [details] [review]
Link to github

Some nit addressed, hope we can land this patch in this sprint and create follow up if necessary.
Attachment #8524519 - Flags: review?(felash)
(Assignee)

Comment 6

3 years ago
Created attachment 8526640 [details]
rtl.jpg

Hi Jenny, it's about the UI display for RTL text thread list under LTR environment. It sounds reasonable to set the direction depends on the text content, but it might look weird if we only move the recipient and message text to right and leave others at left. Ahmed, we will need you opinion from Arabic user POV to know if it's necessary to change for this case, thanks.
Flags: needinfo?(nefzaoui.ahmed)
Flags: needinfo?(jelee)
(Assignee)

Comment 7

3 years ago
Created attachment 8526642 [details]
rtl-content.jpeg

It's the result from current patch if the message contains both LTR and RTL text. We set different direction in bubble depend on text ,and we thought it's the best approach for now. Please let us know if you prefer unified direction for whole messages based on the system language or unified direction per message based on the first word of message, thanks.
(Reporter)

Comment 8

3 years ago
Apologies for my late reply. My only concern for now here is the use of -moz-dir() for the outgoing and incoming bubbles. If we are *ONLY* mirroring their positions shouldn't we just use html[dir="rtl"] ?

(In reply to Steve Chung [:steveck] from comment #6)
> Created attachment 8526640 [details]
> rtl.jpg
> 
> Hi Jenny, it's about the UI display for RTL text thread list under LTR
> environment. It sounds reasonable to set the direction depends on the text
> content, but it might look weird if we only move the recipient and message
> text to right and leave others at left. Ahmed, we will need you opinion from
> Arabic user POV to know if it's necessary to change for this case, thanks.

I think we should just keep it that way. Don't know about what would UX say but if we're gonna think about handling both RTL and LTR cases for every single direction, we should probably mirror all the elements of a single message depending on the text content (time, date, number/recipient etc), but that then will create confusion as the user start to have RTL and non-RTL messages and will create more complex use cases. (hope my explanation is clear enough? :S)

(In reply to Steve Chung [:steveck] from comment #7)
> Created attachment 8526642 [details]
> rtl-content.jpeg
> 
> It's the result from current patch if the message contains both LTR and RTL
> text. We set different direction in bubble depend on text ,and we thought
> it's the best approach for now. Please let us know if you prefer unified
> direction for whole messages based on the system language or unified
> direction per message based on the first word of message, thanks.

From my POV: PERFECT.
Yes, it may look weird (not so much though) but from a readability perspective it's what we should actually be doing...
Flags: needinfo?(nefzaoui.ahmed)
Ahmed, if you can try the patch in real conditions on your phone, it would be awesome :)
Of course it could be done later as well, and you could file follow-up bugs if you think things could get improved.

NI Ahmed for this, but take your time!
Flags: needinfo?(nefzaoui.ahmed)
Comment on attachment 8524519 [details] [review]
Link to github

I'm afraid this doesn't work properly with -moz-plaintext once we have RTL and LTR text in the same sentence. We need to find other solutions :/
Attachment #8524519 - Flags: review?(felash)
(In reply to Steve Chung [:steveck] from comment #6)
> Created attachment 8526640 [details]
> rtl.jpg
> 
> Hi Jenny, it's about the UI display for RTL text thread list under LTR
> environment. It sounds reasonable to set the direction depends on the text
> content, but it might look weird if we only move the recipient and message
> text to right and leave others at left. Ahmed, we will need you opinion from
> Arabic user POV to know if it's necessary to change for this case, thanks.

(In reply to Steve Chung [:steveck] from comment #6)
> Created attachment 8526640 [details]
> rtl.jpg
> 
> Hi Jenny, it's about the UI display for RTL text thread list under LTR
> environment. It sounds reasonable to set the direction depends on the text
> content, but it might look weird if we only move the recipient and message
> text to right and leave others at left. Ahmed, we will need you opinion from
> Arabic user POV to know if it's necessary to change for this case, thanks.



What Oleg did for recipients is having "text-align: right" if parent is ":-moz-dir(rtl)" (and "text-align: left" otherwise). I don't know if that could help you here though.
(Assignee)

Comment 12

3 years ago
Created attachment 8527472 [details]
moz-plaintext-ltr.jpeg

If the a sentence contains bot LTR and RTL language and starting word's direction is different from the system's direction, I admit that it looks weird if we force the whole sentence in same direction. But if the different direction word is show up right in the middle, it becomes even more weird for my POV. I'll attach another example later. Ni? Ahmed for the correct display direction for LTR/RTL mix up.
Attachment #8527472 - Flags: feedback?(nefzaoui.ahmed)
(Assignee)

Comment 13

3 years ago
Created attachment 8527476 [details]
moz-plaintext.jpeg

It's another example about the RTL/LTR mix up. The display with -moz-plaintext will force the sentence's direction defined by the first word of sentence. Still NI? Ahmed for the correct display direction.

Comment 14

3 years ago
Hey Steve, I think the reading experience should be as intuitive as possible for RTL readers, even if we need to slightly compromise on the visual aspect. let's follow what Ahmed said here. thanks!
Flags: needinfo?(jelee)
(Assignee)

Updated

3 years ago
Blocks: 1104715
(Assignee)

Updated

3 years ago
No longer blocks: 1104715
Whiteboard: [p=2] → [p=2 for 2.1S9][p=1 for 2.2S1]
Target Milestone: 2.1 S9 (21Nov) → 2.2 S1 (5dec)

Comment 15

3 years ago
Ahmed, we need feedback to determine if this can land in 2.2 or not.
(Reporter)

Comment 16

3 years ago
Sorry for the way-too-much late reply :(

(In reply to Steve Chung [:steveck] from comment #13)
> Created attachment 8527476 [details]
> moz-plaintext.jpeg
> 
> It's another example about the RTL/LTR mix up. The display with
> -moz-plaintext will force the sentence's direction defined by the first word
> of sentence. Still NI? Ahmed for the correct display direction.

Concerning this:
with -moz-plaintext in RTL: perfectly correct. :)
without -moz-plaintext in RTL: quite very messed up, should not be like this.
with -moz-plaintext in LTR: Also correct.
without -moz-plaintext in LTR: Not correct.

(In reply to Steve Chung [:steveck] from comment #12)
> Created attachment 8527472 [details]
> moz-plaintext-ltr.jpeg
> 
> If the a sentence contains bot LTR and RTL language and starting word's
> direction is different from the system's direction, I admit that it looks
> weird if we force the whole sentence in same direction. But if the different
> direction word is show up right in the middle, it becomes even more weird
> for my POV. I'll attach another example later. Ni? Ahmed for the correct
> display direction for LTR/RTL mix up.

The example with -moz-plaintext makes much more sense since the messages starts with Arabic, logically the whole content starts right and then aligns to whatever we have else within the message. So yes, with -moz-plaintext is LTR is the correct one :)

(In reply to Steve Chung [:steveck] from comment #7)
> Created attachment 8526642 [details]
> rtl-content.jpeg
> 
> It's the result from current patch if the message contains both LTR and RTL
> text. We set different direction in bubble depend on text ,and we thought
> it's the best approach for now. Please let us know if you prefer unified
> direction for whole messages based on the system language or unified
> direction per message based on the first word of message, thanks.

From my perspective, we should do different text direction depending on text. So attachment id=8526642 makes perfect sense :)
Flags: needinfo?(nefzaoui.ahmed)
(Reporter)

Comment 17

3 years ago
Comment on attachment 8524519 [details] [review]
Link to github

Looking good, tried every possible case and it looks like everything is working fine.
However, I'm finding another bug a little annoying; It would be better if we have the text field aligned to the direction of the first character written in it :)
Attachment #8524519 - Flags: feedback?(nefzaoui.ahmed) → feedback+
(Assignee)

Comment 18

3 years ago
(In reply to Ahmed Nefzaoui [:Nefzaoui] from comment #17)
> Comment on attachment 8524519 [details] [review]
> Link to github
> 
> Looking good, tried every possible case and it looks like everything is
> working fine.
> However, I'm finding another bug a little annoying; It would be better if we
> have the text field aligned to the direction of the first character written
> in it :)

Thanks for the feedback! Could you please clarify the issue you faced? Maybe a image for discription would be better. We might fix right in this patch if issue is not difficult, otherwise we can fire another bug for it.
Flags: needinfo?(nefzaoui.ahmed)
(Assignee)

Comment 19

3 years ago
Comment on attachment 8524519 [details] [review]
Link to github

Hi Oleg, the patch would works fine based on the Ahmed's feedback(except thte one issue he mentioned above). Since Julien might busy on other stuff and you also have lots of experience on RTL, could you please take a final look for it? Thanks!
Attachment #8524519 - Flags: review?(azasypkin)
Comment on attachment 8524519 [details] [review]
Link to github

Looks good to me, thanks!
Attachment #8524519 - Flags: review?(azasypkin) → review+
(Assignee)

Comment 21

3 years ago
Thanks!
In master: 1672a719e5cac08f51beb92ada4670066c0cf9c7
Status: NEW → RESOLVED
Last Resolved: 3 years ago
Resolution: --- → FIXED
Duplicate of this bug: 1115081
(Reporter)

Updated

3 years ago
Flags: needinfo?(nefzaoui)
Attachment #8524519 - Flags: feedback+
(Reporter)

Updated

3 years ago
Attachment #8527472 - Flags: feedback?(nefzaoui)
You need to log in before you can comment on or make changes to this bug.