Page MenuHomePhabricator

Popups leave a large margin between thumbnail and description when thumbnail is narrow
Closed, ResolvedPublic5 Estimated Story Points

Description

NOTE: this tasks refers to the width of the popup, not the height.

User Story

  • When I hover over a link with a narrow thumbnail, ex: Holy Roman Emperor, or The Barker there's a large margin between the thumbnail and the text extract.
Screen Shot 2018-04-24 at 17.36.29.png (299×505 px, 141 KB)
large margin
Screen Shot 2018-04-24 at 17.37.12.png (287×517 px, 165 KB)
small margin

Notice the different distance from the settings cog to the image. The popup should be made less wide when the thumbnail is narrow.

Desire result

The width of the popup should change if the thumbnail is narrow.

Screen Shot 2018-04-24 at 17.36.29.png (299×505 px, 141 KB)
now , 450px wide
Screen Shot 2018-04-24 at 18.00.21 1.png (295×475 px, 145 KB)
better? less than 450px wide

Why is this a problem?

This change would enable us to solve T191267 without a substantial rewrite of the template markup. The easiest solution to T191267 (adding a border between the thumbnail and description) is to place a border-right on the text-extract with CSS. This approach doesn't work when the SVG is a constant width but the thumbnail is a variable width.

Screen Shot 2018-04-19 at 19.28.15.png (320×513 px, 133 KB)

This border would work if the popup was less wide.

Design

Add a 1px border of rgba(0,0,0,0.1) to the bottom or side edge of image (depending on orientation) in the page preview. The border should be overlapping the image. Here is a diagram to clarify the placement of the border:

pagepreview-imageborder.jpg (751×1 px, 149 KB)

Here are some examples that approximate the design, rendered in the browser via common.css:

Screen Shot 2018-04-05 at 12.40.18 PM.png (920×836 px, 460 KB)

Screen Shot 2018-04-05 at 12.40.41 PM.png (540×986 px, 550 KB)

Screen Shot 2018-04-05 at 12.38.22 PM.png (894×1 px, 487 KB)

Screen Shot 2018-04-05 at 12.39.21 PM.png (898×832 px, 235 KB)

Screen Shot 2018-04-05 at 12.41.02 PM.png (858×732 px, 242 KB)

Note:

  • make sure this works on RTL previews

Developer notes

As we discovered the svg mask means we cannot use border-left but we can use a polyline element to achieve this, making sure it is horizontal or vertical and aligned with respect to the pokey.

QA Notes

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes

@alexhollender does this need any more design input or is this ready for developer analysis?

ovasileva triaged this task as Medium priority.May 2 2018, 1:54 PM
ovasileva set the point value for this task to 8.May 9 2018, 4:16 PM

Just had an idea of a compromise.. will explore today to see if it's feasible and report back.

Change 440645 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Add SVG border using polyline element

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

Jdlrobson removed the point value for this task.Jun 16 2018, 12:01 AM

Okay, I spent an afternoon looking into this and I've come up with a solution using SVG polyline element ^^^^. Hoping we can talk about the merits/suckiness of the solution and throw some points on it if we want to go ahead with it.

ovasileva set the point value for this task to 5.Jun 19 2018, 4:12 PM
ovasileva moved this task from Upcoming to 2017-18 Q4 on the Web-Team-Backlog board.

@Jdlrobson This looks promising! The patch doesn't yet account for thumbnails that are narrower than the maximum width (i.e. the Holy Roman Emperor example), but that use-case is easily addressed because we can position the line at the same x value as the thumbnail. II haven't tested in RTL yet, but in general I think we can move forward with this polyline solution ( I don't think their should be any browser-compatiliby issues either).

@Jdrewniak I'm not seeing the issues you are seeing on the existing patch can you give me some clearer replication instructions?
this is what I'm seeing...

Screen Shot 2018-06-21 at 5.37.29 PM.png (493×801 px, 331 KB)

Screen Shot 2018-06-21 at 5.35.58 PM.png (478×649 px, 279 KB)

I think that you mean there shouldn't be whitespace to the left of the image? On second thoughts.. with narrow images, shouldn't this whitespace be eradicated (cc @alexhollender ) ?

with narrow images, shouldn't this whitespace be eradicated

I was under the impression that removing the whitespace, thus making the entire preview more narrow, was the goal here yes

@alexhollender Originally, in this task description, making the the entire preview more narrow was half of a (my) two part solution:

  1. make the entire preview narrow when necessary (this task).
  2. add some sort of css border. (discussed in this task T191267).

With @Jdlrobson's approach however (placing a polyline element inside the svg) and per my comment here, we don't have to make the entire preview more narrow to achieve the desired effect. With this approach we can instead position the polyline element to same x value as the edge of the thumbnail. From an engineering perspective, I think this might be easier than fiddling with the CSS.

So just to be absolutely clear, the only remaining issue I see with @Jdlrobson's current patch is this:

Screen_Shot_2018-06-21_at_5.35.58_PM.png (478×649 px, 286 KB)

Now, from a design perspective, I guess we might still want to get rid of the extra white-space anyway... maybe in a followup patch after the border has been added?

@Jdrewniak thanks for the clarification. Yes, I think either here (or in a separate task) we should get rid of the extra whitespace. Let me know if you'd like me to set that up.

Change 442199 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Extracts can expand with narrow thumbnails

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

@Jdlrobson nice! Can we also get the settings icon to move over in the case that we move forward with that solution?

feck. Completely became blind to the settings cog... !

┈┈┈┈┈┈▕▔╲
┈┈┈┈┈┈┈▏▕
┈┈┈┈┈┈┈▏▕▂▂▂
▂▂▂▂▂▂╱┈▕▂▂▂▏
▉▉▉▉▉┈┈┈▕▂▂▂▏
▉▉▉▉▉┈┈┈▕▂▂▂▏
▔▔▔▔▔▔╲▂▕▂▂▂I

These look great. A huge improvement over what was before. Good work all.

Change 440645 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Add SVG border using polyline element

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

Change 442199 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Extracts can expand with narrow thumbnails

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

Vvjjkkii renamed this task from Popups leave a large margin between thumbnail and description when thumbnail is narrow to 7beaaaaaaa.Jul 1 2018, 1:14 AM
Vvjjkkii removed alexhollender_WMF as the assignee of this task.
Vvjjkkii raised the priority of this task from Medium to High.
Vvjjkkii updated the task description. (Show Details)
Vvjjkkii removed the point value for this task.
Vvjjkkii removed subscribers: gerritbot, Aklapper.
Mainframe98 renamed this task from 7beaaaaaaa to Popups leave a large margin between thumbnail and description when thumbnail is narrow.Jul 1 2018, 7:45 AM
Mainframe98 assigned this task to alexhollender_WMF.
Mainframe98 lowered the priority of this task from High to Medium.
Mainframe98 updated the task description. (Show Details)
Mainframe98 set the point value for this task to 5.
Mainframe98 added subscribers: gerritbot, Aklapper.

Looks good to me on Chrome, Safari, Firefox, Edge, and IE

Screen Shot 2018-07-03 at 9.52.29 AM.png (528×922 px, 479 KB)

Screen Shot 2018-07-03 at 9.53.06 AM.png (560×930 px, 450 KB)

Screen Shot 2018-07-03 at 9.55.18 AM.png (570×928 px, 448 KB)

These are looking so nice :) added to release timeline.

Screen Shot 2018-07-04 at 12.45.06 PM.png (391×459 px, 93 KB)