Page MenuHomePhabricator

Review RTL issues with Education dashboard
Closed, ResolvedPublic3 Story Points

Description

On a program's Overview page, opening the Review article button opens the modal off the left side of the screen, rather than to the right. See pics.


In RTL, there's a clickable box near the top left. When you click on it, a menu pops up: Username, MS Chars Added, US Chars Added. I'm not sure where it comes from.

There are several parentheses failures:

When you work on this stuff -- Moriel in the Collaboration team is our RTL rock star. Definitely ping her if you've got questions.

Event Timeline

awight created this task.Feb 18 2016, 7:45 PM
Restricted Application added subscribers: StudiesWorld, Base, Aklapper. · View Herald TranscriptFeb 18 2016, 7:45 PM
awight renamed this task from Review RTL issues to Spike: Review RTL issues with Education dashboard.Feb 27 2016, 6:37 AM
awight updated the task description. (Show Details)
DannyH renamed this task from Spike: Review RTL issues with Education dashboard to Review RTL issues with Education dashboard.Sep 20 2016, 6:19 PM
DannyH triaged this task as Normal priority.
DannyH added a project: Community-Tech.
DannyH moved this task from Untriaged to To be estimated/discussed on the Community-Tech board.
DannyH updated the task description. (Show Details)Sep 20 2016, 7:02 PM
DannyH updated the task description. (Show Details)Sep 20 2016, 7:17 PM
DannyH updated the task description. (Show Details)
kaldari set the point value for this task to 3.
Niharika claimed this task.Oct 6 2016, 12:25 PM
Niharika moved this task from Ready to In Development on the Community-Tech-Sprint board.

@Ragesoss Do you have any tips for getting started on fixing this? Almost all popovers are misplaced in RTL.

@Niharika: I don't have much in the way of tips. @awight set up the RTL initially, but we never did anything special besides use a tool to generate flipped stylesheets as part of the i18n framework. So in this case, I'm guessing that we're doing something with the css for positioning the popovers that doesn't play nicely with the stylesheet flipper. Ideally, we could find an alternative way to style the popovers that leaves the LTR rendering as it is now, but works correctly with RTL. But my brief look at the styles for the popovers didn't enlighten me very much on how to fix it.

Typically RTL flipping systems work by looking for things like margin-right and changing them to margin-left or they look for special commands embedded in CSS comments. Maybe @awight can give us some guidance on how this one works.

Niharika removed Niharika as the assignee of this task.Oct 27 2016, 2:56 PM
Niharika moved this task from In Development to Ready on the Community-Tech-Sprint board.
Niharika added a subscriber: Niharika.
Mardetanha added a subscriber: Mardetanha.
DStrine removed a subscriber: awight.Oct 31 2016, 2:16 PM
kaldari lowered the priority of this task from Normal to Low.Nov 16 2016, 6:07 AM

This PR would solve some parts. I think you see how that can be applied to other problems: https://github.com/WikiEducationFoundation/WikiEduDashboard/pull/1036

It looks like neither @noflip nor @replace will work directly to fix the modals from the first screenshot. It looks like this is because the css compiler turns one transform instruction into a pair of them — -webit-transform and transform — and the css-flipper annotation only applies to the first one.

Just a quick note:
Here's the problem:


By changing the "right" in them to "left" all issues gets solved:

css-flip says by adding noflip you can prevent this change. If this note in css-flip doesn't work that's really strange.

I think the problem is that our build process first takes the .styl file and turns it into css, and then applies css-flipper.

The output of the first step can turn one css statement into two, but the css-flipper annotation only appears for the first one. So the end result is something like this:

/*@replace: translateX(0) translateY(0) scale(.7, .7)*/

-webkit-transform: translateX(0) translateY(0) scale(.7, .7);
transform: translateX(-50%) translateY(-30px) scale(0.7, 0.7);

I'll report this upstream to stylus, but we may need to find a workaround.

Re: stylus, they suggest looking elsewhere in our css compilation pipeline: https://github.com/stylus/stylus/issues/2248

Looks like we can work around it by turning autoprefixer off and then explicitly doing both transform and -webkit-transform. Something like this for .pop, although the values aren't right yet:

`

/* autoprefixer: off */
/*@replace: translateX(0) translateY(0) scale(.7, .7)*/ transform translateX(-50%) translateY(-30px) scale(.7, .7)
/*@replace: translateX(0) translateY(0) scale(.7, .7)*/ -webkit-transform translateX(-50%) translateY(-30px) scale(.7, .7)
/* autoprefixer: on */
transition all .2s
z-index 10
&.open
  opacity 1
  pointer-events all
  /* autoprefixer: off */
  /*@replace: translateX(0) translateY(0) scale(1, 1)*/ transform translateX(-50%) translateY(0) scale(1, 1)
  /*@replace: translateX(0) translateY(0) scale(1, 1)*/ -webkit-transform translateX(-50%) translateY(0) scale(1, 1)
  /* autoprefixer: on */

`

Got it!

That takes care of the first two screenshots.

The menu should be fixable with one of the same techniques.

The parentheses issue only appears because those messages are not yet translated and the English fallbacks are being used. Should be fine once translated.

The remaining issue with the menu is not a positioning problem; it's that all the icons are not appearing in RTL. That's because the RTL stylesheet is not in the same directory as the default one, so relative links to images in the RTL stylesheet are broken.

Got it, and deployed to P&E Dashboard! That's all the known RTL problems, I think.

Thanks for showing the way, @Ladsgroup!

Ragesoss closed this task as Resolved.Nov 20 2016, 10:27 PM
Ragesoss claimed this task.

Please reopen if there are additional RTL issues that turn up.