Page MenuHomePhabricator

[Bug] Page preview portrait images should be flipped in right-to-left languages
Closed, ResolvedPublic1 Story Points

Description

Steps to reproduce

  1. Visit https://he.wikipedia.org
  2. Enable page previews
  3. Visit https://he.wikipedia.org/wiki/%D7%9E%D7%A9%D7%9C%D7%99?uselang=he
  4. Hover over שלמה

Expected results

  • Portrait images appear on the left for RTL languages

Actual results

  • Portrait images appear on the right for RTL languages


Environments observed

  • he.wikipedia.org

Browser Version:

  • Chromium v64.0.3282.167

OS Version:

  • Ubuntu 17.10 64b

Device Model:

  • Desktop

Device Language:

  • English

Developer notes

The following rule should be all thats needed

a.mwe-popups-discreet {
    float: right;
}

CSS Janus will flip floats by default, so this will cause images to float left on Hebrew.
This will not impact page previews where the image is on top.

Acceptance critera

  • Portrait images appear on the left or TOP for RTL languages
  • Portrait images appear on the right or TOP for LTR languages

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 27 2018, 3:26 PM

Im a little confused as this looks like it's working correctly for me.

The pokey will always point to the link, it's not about LTR or RTL. For instance in the screenshot I'm posting the pokey appears on the right (as that's where the link is) and the image is displayed on the left. It's impossible for that pokey to be on the left without moving the page preview over the navigation menu.

It looks like your content language is set to RTL and your UI language is set to LTR which is causing the problem. Page previews honors the content language NOT ui language which is probably how it should work.

@Nikerabbit @Amire80 any thoughts on how this should work?

ovasileva renamed this task from [Bug] Page preview landscape images and pokey should be flipped in right-to-left languages to [Bug] Page preview portrait images should be flipped in right-to-left languages.Apr 2 2018, 11:54 AM
ovasileva updated the task description. (Show Details)

The default location for images on desktop is the end: right-hand side for LTR languages and left-hand side for LTR languages. This location is quite often overridden by editors, but most images use the default placement.

Furthermore, infoboxes are pretty much always in the end, and the image in the infobox is often the same as the image in page preview.

So page previews should follow the same logic: The image should be in the end.

Restricted Application added a project: I18n. · View Herald TranscriptApr 2 2018, 11:55 AM
Amire80 moved this task from Untriaged to RTL on the I18n board.Apr 2 2018, 11:56 AM
ovasileva triaged this task as Normal priority.Apr 2 2018, 11:57 AM
ovasileva added subscribers: Nirzar, ovasileva.

Just spoke with @Amire80. His suggestion was to switch the image to the left side for RTL languages. I've updated the task description based on this. @Nirzar - any concerns?

Nirzar added a comment.Apr 2 2018, 4:56 PM

no concerns. sounds good to me.

Jdlrobson added a project: good first bug.
Jdlrobson updated the task description. (Show Details)
Jdlrobson set the point value for this task to 1.Apr 3 2018, 4:31 PM
Niedzielski moved this task from To Do to Doing on the Readers-Web-Kanbanana-Board-Old board.

Change the UI language to Hebrew

This is a little confusing. If you're on Hebrew Wikipedia this will already be the case and you can't change it...?
Note confusingly, if https://en.wikipedia.org?uselang=he the content language is still English so portrait images should continue to appear on the left in that case.

Thanks @Jdlrobson. Since the user may have their preference set to show the UI in English / LTR, I've updated the URL and removed this step.

@Niedzielski did you find some hidden complexity? Can you share?

Was this the z-index issue you were talking about?


@Jdlrobson, there's more to it than that but if you're really curious, I'm happy to chat in depth.

^ In case you're interested, we use an SVG mask in the pokey and that mask needs to be different in LTR/RTL. Looks like it won't impact T191267 in any way.

Change 425737 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Hygiene: consolidate clip-path manipulation

https://gerrit.wikimedia.org/r/425737

Change 425738 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Fix: show thumbnails on left for right-to-left UIs

https://gerrit.wikimedia.org/r/425738

Niedzielski removed Niedzielski as the assignee of this task.Apr 12 2018, 1:54 AM

Change 425737 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: consolidate clip-path manipulation

https://gerrit.wikimedia.org/r/425737

Change 426134 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Hygiene: consolidate CSS class manipulation

https://gerrit.wikimedia.org/r/426134

Change 426970 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Fix: unwanted thumbnail spacing in RTL locales

https://gerrit.wikimedia.org/r/426970

Paging @Mooeypoo @Amire80 for some RTL advice!

Currently image thumbnails appear on the right of page previews (e.g. hover over בעלי החיים):
https://he.wikipedia.org/w/index.php?title=%D7%9C%D7%95%D7%95%D7%99%D7%99%D7%AA%D7%A0%D7%99_%D7%9E%D7%96%D7%99%D7%A4%D7%95%D7%AA&useskin=vector

We're proposing moving the thumbnails to the left:
http://reading-web-staging.wmflabs.org/w/index.php?title=%D7%9C%D7%95%D7%95%D7%99%D7%99%D7%AA%D7%A0%D7%99_%D7%9E%D7%96%D7%99%D7%A4%D7%95%D7%AA

Does this behaviour look right to you both? Does anything look out of place?

I walked @Mooeypoo through this.
Apparently the existing view does not look strange or broken from an i18n perspective and she wouldn't have noticed there was any problem had we not raised it. It only becomes broken when you know how English Wikipedia looks and thus from a consistency purpose. Moriel points out that it's only broken from a design perspective if @Nirzar has decided it is extremely important for the summary to appear before the image.

The new version as implemented by @Niedzielski works well so can be merged.

I'm going to wait until the branch is cut before merging the change.

@Niedzielski should I hold off on merging https://gerrit.wikimedia.org/r/#/c/425738/ ? I was going to do so Tuesday, but it was moved to doing so I'm not sure if it's not ready.

Change 426134 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: consolidate CSS class manipulation

https://gerrit.wikimedia.org/r/426134

I walked @Mooeypoo through this.
Apparently the existing view does not look strange or broken from an i18n perspective and she wouldn't have noticed there was any problem had we not raised it. It only becomes broken when you know how English Wikipedia looks and thus from a consistency purpose.

... And a lot of people who read Wikipedia in an RTL language, also read Wikipedia in an LTR language, so yes, consistency is important.

Moriel points out that it's only broken from a design perspective if @Nirzar has decided it is extremely important for the summary to appear before the image.

It's indeed best not to talk about "right" and "left", but to talk about "before" and "after". My intuition tells me that the text should be before the image in horizontal orientation because that's the default orientation of usual articles.

I have no opinion about the vertical orientation, i.e. whether the image should be above or below the text.

@Jdlrobson, good to go from my perspective. I had it in doing to signify the follow up refactors I did but forgot to move it into review.

Niedzielski removed Niedzielski as the assignee of this task.Apr 25 2018, 10:43 PM

Please test on beta cluster for ltr and rtl!
E.g.
https://en.wikipedia.beta.wmflabs.org/
https://he.wikipedia.beta.wmflabs.org/
Lemme know if you need any th ing.
Leave at least 30 minutes from now (wed pm) before testing.

Change 425738 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Fix: show thumbnails on left for right-to-left UIs

https://gerrit.wikimedia.org/r/425738

Change 429186 had a related patch set uploaded (by Niedzielski; owner: Stephen Niedzielski):
[mediawiki/extensions/Popups@master] Hygiene: replace okies with ointers

https://gerrit.wikimedia.org/r/429186

Change 426970 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Fix: unwanted thumbnail spacing in RTL locales

https://gerrit.wikimedia.org/r/426970

This looks good on beta aside from a problem on the Mac/Firefox combination, but that is not specific to RTL. I'll write up a separate ticket for that.

Note - I saw that Nirzar was working on this ticket previously. Sending to design review for a quick look-see. I used this article and hovered over the blue link in the second paragraph: https://he.wikipedia.beta.wmflabs.org/wiki/%D7%A7%D7%A8%D7%9C_%D7%90%D7%A0%D7%98%D7%95%D7%9F

Nevermind the Mac/Firefox issue. It seems to have gone away.

Change 429186 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Hygiene: replace okies with ointers

https://gerrit.wikimedia.org/r/429186

Jdlrobson added subscribers: alexhollender, ABorbaWMF.

@alexhollender can this be moved to sign off?

Looks good. Moving to sign off b/c Anthony already QA'd.

ovasileva closed this task as Resolved.May 2 2018, 2:01 PM

looks good to me too! tested the same article @ABorbaWMF did.