Closed Bug 1216496 Opened 6 years ago Closed 6 years ago

(Gaia RTL 2.5) CSS refactoring: email

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(b2g-master fixed)

RESOLVED FIXED
Tracking Status
b2g-master --- fixed

People

(Reporter: autra, Assigned: autra)

References

Details

Attachments

(1 file)

RTL CSS refactoring for the email application.
Assignee: nobody → augustin.trancart
Depends on: 1217474
Status: NEW → ASSIGNED
Comment on attachment 8678133 [details] [review]
[gaia] Phoxygen:bug1216496-email_rtl_refactoring > mozilla-b2g:master

Hi James, please R?
Attachment #8678133 - Flags: review?(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.
Attachment #8678133 - Flags: review?(jrburke)
Thanks James!

I've left a question about 1) and an explanation about 2).
Flags: needinfo?(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.
Flags: needinfo?(jrburke)
Comment on attachment 8678133 [details] [review]
[gaia] Phoxygen:bug1216496-email_rtl_refactoring > mozilla-b2g:master

Hey James, here is another try.
please r? again!
Attachment #8678133 - Flags: review?(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.
Attachment #8678133 - Flags: review?(jrburke) → review+
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.
NI myself to land that once tests are green.
Flags: needinfo?(augustin.trancart)
putting checkin-needed, but note that GU21 is permafail apparently.
Flags: needinfo?(augustin.trancart)
Keywords: checkin-needed
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.
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.