1216496 - (Gaia RTL 2.5) CSS refactoring: email

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[Augustin Trancart [:autra]]

RTL CSS refactoring for the email application.

Comment 1

[GitHub Autolander Bot]

Attachment: [gaia] Phoxygen:bug1216496-email_rtl_refactoring > mozilla-b2g:master

Comment 2

[Augustin Trancart [:autra]]

Comment on attachment 8678133 [details] [review]
[gaia] Phoxygen:bug1216496-email_rtl_refactoring > mozilla-b2g:master

Hi James, please R?

Comment 3

[James Burke [:jrburke]]

Comment on attachment 8678133 [details] [review]
[gaia] Phoxygen:bug1216496-email_rtl_refactoring > mozilla-b2g:master

Looking good, the two things to sort out, as mentioned in pull request comments:

1) Remove the gaia-icons if possible, let's use a separate bug and pull request since it likely means adding more icons to the font.

2) There was a case where it moved away from dir="auto" to CSS styles. If it is something special just for that element it would be good to understand more. Otherwise, should the rest of the dir="auto" elements also be converted?

Flip review back when it is all sorted, feel free to ask questions in the pull request and I will respond over there.

Comment 4

[Augustin Trancart [:autra]]

Thanks James!

I've left a question about 1) and an explanation about 2).

Comment 5

[James Burke [:jrburke]]

Feedback in the pull request, thanks for taking the time to get me up to speed on the issues! Flip review back to me when ready.

Comment 6

[Augustin Trancart [:autra]]

Comment on attachment 8678133 [details] [review]
[gaia] Phoxygen:bug1216496-email_rtl_refactoring > mozilla-b2g:master

Hey James, here is another try.
please r? again!

Comment 7

[James Burke [:jrburke]]

Comment on attachment 8678133 [details] [review]
[gaia] Phoxygen:bug1216496-email_rtl_refactoring > mozilla-b2g:master

Looks good! I can see where the bubbles used on message reader and compose could lay out a mixed bidi email address differently than what is used in the rest of the app, but given the constraints that they could also just be regular names, not email addresses, and we want to avoid bugs around ellipsis issues, this seems like a good tradeoff.

Comment 8

[Augustin Trancart [:autra]]

Oh I forgot about the bubble! But you're right, we cannot put them on ltr direction in css, we need to have it in js I guess. 

I'm opening a followup for this.

Comment 9

[Augustin Trancart [:autra]]

NI myself to land that once tests are green.

Comment 10

[Augustin Trancart [:autra]]

putting checkin-needed, but note that GU21 is permafail apparently.

Comment 11

[Sam Foster [:sfoster] (he/him)]

Merged to master: https://github.com/mozilla-b2g/gaia/commit/7607cb7492ca9063da8f2a8719706d68eab5d451

The Gu21 failure is unrelated here and seems to be a permafail, I've seen that on lots of other PR recently.