Create Task
Maniphest T246029

Increase max-height of reference previews
Closed, ResolvedPublic1 Estimated Story Points
Actions

Description

Description

Based on user feedback and the data available in this dashboard (link), we've decided to increase the max-height of reference previews. The intended goal is to reduce the percentage of reference previews that need a scrollbar, while still maintaining a max-height and visual parity with page previews.

Design

The new max-height of the entire preview should match the max-height of page previews (roughly 403px). For the purpose of the screenshot below this was achieved my increasing the max-height of .mwe-popups-extract to 292px.

image.png (518×940 px, 304 KB)

Details

SubjectRepoBranchLines +/-
Resize popup to 403pxmediawiki/extensions/Popupsmaster+5 -6
Increase maximum height for reference previewsmediawiki/extensions/Popupsmaster+2 -3
Customize query in gerrit

Related Objects
Search...

StatusSubtypeAssignedTask
 ResolvedthiemowmdeT246029 Increase max-height of reference previews
 ResolvedNoneT241750 Consider extending popup size

Event Timeline

• alexhollender_WMF created this task.Feb 24 2020, 6:36 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 24 2020, 6:36 PM
• alexhollender_WMF updated the task description. (Show Details)Feb 24 2020, 6:36 PM
Lena_WMDE moved this task from Backlog to Before Default on the Reference Previews board.Feb 25 2020, 8:43 AM
Charlie_WMDE moved this task from Incoming to IncomingTCB on the WMDE-Design board.Apr 6 2020, 11:14 AM
• alexhollender_WMF mentioned this in T249530: Move scrollbar to outer container.Apr 6 2020, 6:04 PM
ECohen_WMDE subscribed.Jun 19 2020, 3:13 PM
ECohen_WMDE moved this task from Before Default to Ready for pickup on the Reference Previews board.Jun 24 2020, 8:01 AM
thiemowmde added a subtask: T241750: Consider extending popup size.Jul 22 2020, 7:10 AM
Pols12 awarded a token.Jul 22 2020, 1:23 PM
ECohen_WMDE closed subtask T241750: Consider extending popup size as Resolved.Oct 14 2020, 2:00 PM
ECohen_WMDE mentioned this in T241750: Consider extending popup size.
ECohen_WMDE removed projects: Design, WMDE-Design.Oct 14 2020, 2:03 PM
Jdlrobson added a project: Web-Team-Backlog (Tracking).Oct 15 2020, 8:25 PM
Jdlrobson added subscribers: ovasileva, Jdlrobson.

cc @ovasileva @alexhollender have tagged with tracking as have assumed this is for the WMDE backlog

Lena_WMDE set the point value for this task to 2.Oct 19 2020, 9:10 AM
Lena_WMDE changed the point value for this task from 2 to 1.
Lena_WMDE added a project: WMDE-QWERTY-Sprint-2020-10-21.Oct 21 2020, 8:20 AM
Lena_WMDE moved this task from Ready for pickup to Doing on the Reference Previews board.Oct 21 2020, 8:28 AM
gerritbot added a comment.Oct 27 2020, 1:58 PM

Change 636660 had a related patch set uploaded (by Thiemo Kreuz (WMDE); owner: Thiemo Kreuz (WMDE)):
[mediawiki/extensions/Popups@master] Increase maximum height for reference previews

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

gerritbot added a project: Patch-For-Review.Oct 27 2020, 1:58 PM
thiemowmde claimed this task.Oct 27 2020, 1:58 PM
thiemowmde moved this task from Sprint Backlog to Review on the WMDE-QWERTY-Sprint-2020-10-21 board.
awight moved this task from Review to Watching on the WMDE-QWERTY-Sprint-2020-10-21 board.Oct 28 2020, 9:20 AM
awight moved this task from Watching to Demo on the WMDE-QWERTY-Sprint-2020-10-21 board.
gerritbot added a comment.Oct 28 2020, 11:15 AM

Change 636660 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Increase maximum height for reference previews

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

ReleaseTaggerBot added a project: MW-1.36-notes (1.36.0-wmf.16; 2020-11-03).Oct 28 2020, 12:00 PM
Maintenance_bot removed a project: Patch-For-Review.Oct 28 2020, 12:10 PM
awight subscribed.Oct 29 2020, 10:47 AM

I noticed that there are already mwe-popups-is-tall and mwe-popups-is-not-tall classes which perhaps we should have used here. Our additional style is being applied to a "is not tall" element :-/

thiemowmde added a comment.Oct 29 2020, 11:09 AM

Uh, no, these classes are for something else. "Tall" is for popups that show an image on the right side. They have significantly different size constrains. I believe they are fixed to 450 x 250 pixels (because of the image on the right), while normal popups are 320 pixels wide, and have a flexible height. "Tall" is probably a bad name. It's not the popup that is tall (it's actually wider), but the image.

Reference popups are never tall, because they never show an image.

awight added a comment.Oct 29 2020, 11:30 AM
In T246029#6587939, @thiemowmde wrote:

"Tall" is for popups that show an image on the right side. They have significantly different size constrains. I believe they are fixed to 450 x 250 pixels (because of the image on the right), while normal popups are 320 pixels wide, and have a flexible height.

There is a difference in height, the .mwe-popups-is-tall .mwe-popups-extract selector assigns 9 line heights, and the other assigns 7 line heights. Good points that the width changes, and the intention of the class is to display portrait-aspect thumbnails.

"Tall" is probably a bad name. It's not the popup that is tall (it's actually wider), but the image.

This confirms your analysis: https://github.com/wikimedia/mediawiki-extensions-Popups/blob/master/src/ui/renderer.js#L58

Jdlrobson added a comment.EditedOct 29 2020, 3:35 PM

~~Did this change break https://doc.wikimedia.org/Popups/master/js/ui/?path=/story/thumbnails--landscape-thin-thumbnail and https://doc.wikimedia.org/Popups/master/js/ui/?path=/story/thumbnails--landscape-svg?~~

(I'm just wondering as this code has been historically very brittle and was actually why we setup storybook to monitor these)

Edit: doesn't look like it I'll cut a new bug to investigate...

gerritbot added a comment.Oct 30 2020, 1:03 PM

Change 637690 had a related patch set uploaded (by Awight; owner: Awight):
[mediawiki/extensions/Popups@master] Resize popup to 403px

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

gerritbot added a project: Patch-For-Review.Oct 30 2020, 1:03 PM
thiemowmde closed this task as Resolved.Nov 3 2020, 12:59 PM
Lena_WMDE moved this task from Demo to Done on the WMDE-QWERTY-Sprint-2020-10-21 board.Nov 3 2020, 1:08 PM
gerritbot added a comment.Nov 6 2020, 10:29 AM

Change 637690 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Resize popup to 403px

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

ReleaseTaggerBot edited projects, added MW-1.36-notes (1.36.0-wmf.18; 2020-11-17); removed MW-1.36-notes (1.36.0-wmf.16; 2020-11-03).Nov 6 2020, 11:00 AM
thiemowmde mentioned this in T276200: Fix smaller design issues related to ReferencePreviews settings icon.Mar 2 2021, 9:23 AM
Jdlrobson mentioned this in T255549: Portrait images in PagePreview popups are notably cut off (zoomed in).Aug 6 2021, 4:02 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits