Page MenuHomePhabricator

Popups leave a large margin between thumbnail and description when thumbnail is narrow
Closed, ResolvedPublic5 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.
large margin
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.

now , 450px wide
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.

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:

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





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

Details

Related Gerrit Patches:
mediawiki/extensions/Popups : masterExtracts can expand with narrow thumbnails
mediawiki/extensions/Popups : masterAdd SVG border using polyline element

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
Jdrewniak updated the task description. (Show Details)Apr 25 2018, 9:15 AM
Jdrewniak updated the task description. (Show Details)Apr 25 2018, 9:27 AM
Jdrewniak updated the task description. (Show Details)Apr 25 2018, 9:31 AM
Niedzielski added a subscriber: Niedzielski.

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

All set, let's go for it

Jdlrobson updated the task description. (Show Details)May 1 2018, 10:19 PM
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 Readers-Web-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...

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:

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

After adding polyline: (https://gerrit.wikimedia.org/r/440645)

After fixing thumbnail:

After expanding extract... w00t! (https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/Popups/+/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

Jdlrobson updated the task description. (Show Details)Jun 29 2018, 5:58 PM
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 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.
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.
This comment was removed by alexhollender.

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

ovasileva closed this task as Resolved.Jul 4 2018, 10:47 AM

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