-
Notifications
You must be signed in to change notification settings - Fork 128
Conversation
… in case of RTL languages
This is the initial step in fixing mozilla-services#4261
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.
Phew, I was a little intimidated by all the changes, but once I got into the flow all the CSS changes are the same pattern.
One little issue about the isRtl function, otherwise it's looking really good. Thanks for all this work!
@@ -70,7 +70,7 @@ | |||
@keyframes slide-left { | |||
0% { | |||
opacity: 0; | |||
transform: translate3d(160px, 0, 0); | |||
transform: translate3d(160px, 0, 0); // FIXME: RTL |
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.
Did you intend to leave the FIXME here?
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.
Actually yes.
My experience with CSS animation is not that good, so I was hoping to either learn some more in order to fix it better, or get help form the community.
In any case, I don't think it should stop the merge of this PR, since it only affects the on-boarding flow.
I'm not sure I resolved those package-lock conflicts correctly, but who can ever know? I'll try to regenerate them after this lands. |
Fixes: #4261
Changes include:
dir="rtl"
ordir="ltr"
to<html>
tag according to the browser languagedelete-shot-button
dialog, or correct position for the screenshots pagination.