Maniphest T190831

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

Assigned To

Authored By

	• Niedzielski
	Mar 27 2018, 3:26 PM

Description

Steps to reproduce

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

Expected results

Portrait images appear on the left for RTL languages

Actual results

Portrait images appear on the right for RTL languages

Screenshot from 2018-03-27 10-15-55.png (1×1 px, 495 KB)

Screenshot from 2018-03-27 10-16-13.png (1×1 px, 655 KB)

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

Details

Subject	Repo	Branch	Lines +/-
Hygiene: replace okies with ointers	mediawiki/extensions/Popups	master	+63 -63
Fix: unwanted thumbnail spacing in RTL locales	mediawiki/extensions/Popups	master	+5 -5
Fix: show thumbnails on left for right-to-left UIs	mediawiki/extensions/Popups	master	+238 -122
Hygiene: consolidate CSS class manipulation	mediawiki/extensions/Popups	master	+43 -37
Hygiene: consolidate clip-path manipulation	mediawiki/extensions/Popups	master	+25 -22

Customize query in gerrit

Related Objects
Search...

		Status	Subtype	Assigned	Task
		Resolved		ovasileva	T192622 [EPIC] Page previews post-deploy cleanup
		Resolved		ovasileva	T190831 [Bug] Page preview portrait images should be flipped in right-to-left languages

Event Timeline

• Niedzielski created this task.Mar 27 2018, 3:26 PM

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

• Niedzielski moved this task from Incoming to Needs Prioritization on the Web-Team-Backlog board.Mar 27 2018, 3:26 PM

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

Screen Shot 2018-03-27 at 8.46.29 AM.png (587×1 px, 522 KB)

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

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?

no concerns. sounds good to me.

Jdlrobson moved this task from Needs Prioritization to Upcoming on the Web-Team-Backlog board.Apr 2 2018, 5:46 PM

Jdlrobson added a project: good first task.

Jdlrobson updated the task description. (Show Details)

Jdlrobson set the point value for this task to 1.Apr 3 2018, 4:31 PM

ovasileva added a project: Readers-Web-Kanbanana-Board-Old.Apr 3 2018, 4:32 PM

ovasileva moved this task from Upcoming to 2017-18 Q3 on the Web-Team-Backlog board.Apr 3 2018, 4:41 PM

• Niedzielski claimed this task.Apr 5 2018, 2:32 PM

• Niedzielski moved this task from To Do to Doing on the Readers-Web-Kanbanana-Board-Old board.

• Niedzielski updated the task description. (Show Details)Apr 5 2018, 2:40 PM

MBinder_WMF moved this task from 2017-18 Q3 to 2017-18 Q4 on the Web-Team-Backlog board.Apr 5 2018, 5:26 PM

• Niedzielski updated the task description. (Show Details)Apr 5 2018, 6:40 PM

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.

• Niedzielski updated the task description. (Show Details)Apr 6 2018, 4:33 PM

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.

Screenshot from 2018-04-06 11-34-23.png (697×801 px, 130 KB)

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

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

Screen Shot 2018-04-11 at 4.28.45 PM.png (321×535 px, 89 KB)

Screen Shot 2018-04-11 at 4.28.36 PM.png (365×579 px, 206 KB)

@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

• Niedzielski moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.

• Jhernandez moved this task from Needs Code Review to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.Apr 12 2018, 4:49 PM

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

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-04-17 (1.31.0-wmf.30)).Apr 13 2018, 6:01 PM

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

• Niedzielski moved this task from Needs More Work to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.Apr 18 2018, 5:08 PM

Jdlrobson added a parent task: T192622: [EPIC] Page previews post-deploy cleanup.Apr 20 2018, 12:50 AM

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?

• Niedzielski claimed this task.Apr 23 2018, 5:06 PM

• Niedzielski moved this task from Needs Code Review to Needs More Work on the Readers-Web-Kanbanana-Board-Old board.

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 moved this task from Needs More Work to Doing on the Readers-Web-Kanbanana-Board-Old board.Apr 24 2018, 1:15 AM

@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

ReleaseTaggerBot added a project: MW-1.32-notes (WMF-deploy-2018-05-01 (1.32.0-wmf.2)).Apr 25 2018, 10:00 PM

In T190831#4151747, @Jdlrobson wrote:

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

• Niedzielski moved this task from Doing to Needs Code Review on the Readers-Web-Kanbanana-Board-Old board.

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.

Screen Shot 2018-04-26 at 12.47.38 PM.png (608×1 px, 351 KB)

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