Page MenuHomePhabricator

Truncate page issues
Closed, ResolvedPublic3 Story Points

Description

Update [08/08]
We'd like to wrap up existing work here. There are remaining edge cases, which we'll document and assess separately. JR to talk to Alex about them and work out options.


Background

Sometime issues don't have short descriptions. Sometimes issues with short descriptions still have descriptions that are too long

Currently looks like this:

Acceptance criteria

  • Truncate all page issues (regardless of whether we are displaying a short or a long description) to 2 lines of text at 320px.
  • learn more show appear in the bottom right corner of the issues box (backup plan should be invoked if we spend more than 8 hours trying to make this work - and we should put "Learn more" on its own line).

developer notes

Maybe we need a max-height that limits the element to two lines of text. https://codepen.io/alexhollender/pen/ZROzzE

examples

Here are some pages where issues are running long (on mobile):

sign off

  • document remaining edge cases and follow up with Olga on next steps

estimation notes

When estimating we will want to determine how much more effort it will be to put " learn more" on the 2nd line rather than on a new line. This will help us determine with alex and olga whether it is worth doing.

QA steps

  • Visit a variety of pages with page issues (see list below) and verify that:
    1. the page issue text never exceeds two lines (not including the "Learn more" link)
    2. if your browser width is less than 719px the "Learn more" link is always in the bottom-right corner of the gray banner
    3. if your browser width is greater than 721px the "Learn more" link is always in the bottom-left corner of the gray banner
  • Visit http://reading-web-staging.wmflabs.org/wiki/Transport_in_Brazil, set your browser width to 740px, and verify that the last word of the page issue text is faded out

Here are some pages to use for testing:

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
phuedx renamed this task from 8gaaaaaaaa to Truncate page issues.Jul 2 2018, 4:44 AM
phuedx updated the task description. (Show Details)
phuedx set the point value for this task to 3.
phuedx added a subscriber: Aklapper.

need to check on placement of learn more link with @alexhollender before beginning

alexhollender updated the task description. (Show Details)Jul 3 2018, 5:18 PM

@ovasileva it's now described in the acceptance criteria. this is the thing that @Jdlrobson and @Jdrewniak met about last week.

Jdlrobson updated the task description. (Show Details)Jul 3 2018, 5:20 PM

^ @alexhollender is that timebox fair?

Niedzielski moved this task from To Do to Doing on the Readers-Web-Kanbanana-Board-Old board.
Jdrewniak added a comment.EditedJul 4 2018, 9:59 PM

Not sure if we need more discussion on this task, but as mentioned here https://phabricator.wikimedia.org/T191303#4282504
I have a POC patch that places the "read-more" link on the same line as the content here https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/440556/
I've put this up on http://readers-web-stephen.wmflabs.org for non-technical folks to check out ( ping @alexhollender )

Niedzielski added a subscriber: Niedzielski.

Oops, didn't realize you had something cooking on this! Assigning to you!

Change 445143 had a related patch set uploaded (by Jdrewniak; owner: Jdrewniak):
[mediawiki/skins/MinervaNeue@master] Truncate page-issues to 2 lines of text.

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

Alrighty I think I got something nice. 2 lines of text with the "learn more" link placed on the last line of text. I think it's OK for basic text - tested in a few languages and I haven't come across situations where the line-height was thrown off for some reason* (yet).

@alexhollender the actual width of the fade is open to adjustment :) - right now I set it at 3x the length of 'Learn more' (width:300%) but I haven't seen what that looks like on longer languages yet.

*things that might affect the line-height are super/subscript elements like citations, chemical/math formulas, images, blockquotes, buttons... so basically anthing that isn't plain text 😛

A few bits of feedback on https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/445143/
The fade looks a bit large to me


Especially for italin wikipedia:

and a little disconnected on large devices

yeah the fade shouldn't be x times wide as the text, it should 100% wide + some constant. I've updated the patch to achieve this. The fade is now 4em's longer than the text, and it turns Into solid grey 1em before the text "learn more" starts. Here's a GIF of the resizing behaviour now.

I agree that the effect on large devices is not great, but I don't think we can have it both ways. The 'learn more' text can either be positioned absolutely and always be in the corner, or it can flow with the text, in which case it wouldn't be visible when the text overflows the box.

Also agree. We could however limit the effect and truncation to below the tablet threshold if that makes sense...

@Jdrewniak looking really good! This might be similar to what @Jdlrobson is suggesting, but perhaps we could try using a media query so that for tablet-sized (or larger) screens we can drop Learn more onto its own line, while still capping the page issue text at 2 lines? Here's how it might look with descriptions of various lengths.

Also, how does it look currently with a page issue that only takes up one line on a small screen? http://reading-web-staging.wmflabs.org/wiki/History_of_the_Comoros

Just a quick update on this task. I think it's lookin' classy. @alexhollender I took your advice of using a media query on larger widths, while still truncating to two lines. I think that works pretty nicely!
I used the same breakpoint we use for tablets, (720px) . Here's a video of how it looks with short and long text.

https://drive.google.com/file/d/1HDY9_p9gfckjYHP8aC_gjXU5VvAZ8YN0/view?usp=sharing

@Jdlrobson I think we might even me able to place the "learn more" text with CSS, but I can't figure out how to use ResourceLoaderLessVarFileModule (maybe in a subsequent patch).

🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥
🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥
🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥
🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥
🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥
🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥
🔥 🔥 🔥 🔥 🔥 🔥 🔥 🔥

When viewing on 400 pixel wide device seeing:

on http://localhost:8888/w/index.php/Neutron_activation_analysis

(MobileFrontend=ce34d9be2aceb6ce9164ef4e25dd853e04ef727a,Minerva=1ea68e85eff788c32eef7005d63a9575054219e4)
Looks like, that template has other issues, which are exaggerated by this change:

Also in this case it only displays as one line obscuring the important word "definition":

Jdlrobson updated the task description. (Show Details)Jul 26 2018, 3:04 AM

Change 447971 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Limit page issues truncation to at least 2 lines

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

Jdlrobson added subscribers: nray, Sniedzielski.

Okay, I've been bold and made an edit to the template: https://en.wikipedia.org/w/index.php?title=Template:Copypaste&type=revision&diff=852020473&oldid=784059787&diffmode=source

The second issue as described in https://phabricator.wikimedia.org/T197931#4447263 is addressed by a follow up to Jan's merged patch (https://gerrit.wikimedia.org/r/#/c/mediawiki/skins/MinervaNeue/+/447971). Maybe @nray or @Sniedzielski can review this today so we can push this into Alex's column for design review.

Preempting this event, I've put the patches on reading web staging, so @alexhollender can get a head start.

I left a comment for @Jdrewniak to look at when he gets back but that's not so urgent.

@Jdlrobson, I'm not sure if it's intentional but no fade is performed on large screens (where max-height clips the content).

Change 445143 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Truncate page-issues to 2 lines of text.

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

Change 447971 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Limit page issues truncation to at least 2 lines

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

Jdlrobson added a comment.EditedJul 26 2018, 1:31 PM

~~~Don't think thats intentional (see https://phabricator.wikimedia.org/T197931#4419963)~~~
Scrap that it's still limited to 2 lines so this is okay!

alright here are the two things I'd love to address if possible:

  1. for the situation of a long description + a wide screen, is it possible to keep a fade at the end of the text even once we’ve dropped Learn more to a new line? It should look like this:

  1. is there anything we can do to better handle super short descriptions? Unfortunately no easy fixes come to mind. Hoping that @Jdrewniak might have an ace up his sleeve.


alexhollender added a comment.EditedAug 1 2018, 4:57 PM

@Jdrewniak perhaps we could try ditching the height: 3.3em rule? That would at least make the issues that have less than one line of text look better?


That's weird the height was added in https://gerrit.wikimedia.org/r/445143 to make certain pages with one line of text look better. I thus wouldn't advise removing it. Let me take a closer look.

Jdlrobson added a comment.EditedAug 2 2018, 6:56 AM

<edit: I missed https://phabricator.wikimedia.org/T197931#4469595 which had the context I needed>

Per


@alexhollender don't think it's useful to clip a summary as short as "Some of this article's listed sources may not be reliable." The second part of that sentence is extremely important and it seems unreasonable to limit editors to 5 words, especially in languages where words are much longer.

  1. for the situation of a long description + a wide screen, is it possible to keep a fade at the end of the text even once we’ve dropped Learn more to a new line? It should look like this:

What is the problem we are solving for here given tablet/desktop have much more available space and this is using a template which itself has issues?

Note it looks like this article has changed and I can't find the template that was rendering that, but it looks like the template is not using hidewhencompact
Update: It was using https://en.wikipedia.org/wiki/Template:Academic-written_review which is a very strange ambox template that doesn't fit our current definitions. If we were to update this template to use hidewhencompact, how would it look (I have edit rights)?

alexhollender added a comment.EditedAug 2 2018, 1:18 PM

@Jdlrobson

  1. for the situation of a long description + a wide screen, is it possible to keep a fade at the end of the text even once we’ve dropped Learn more to a new line? It should look like this:

Currently we are clipping the text but not indicating that it is clipped, so it reads like an incomplete sentence. Here is an example from staging:

  1. is there anything we can do to better handle super short descriptions?

Right, so I was less trying to suggest that we do shorter clippings of already short descriptions, and more trying to figure out a way to get the "Learn more" to a less awkward position:

with height definedwithout height defined

Although that comes with the downside of sometimes clipping the last word of a short description. Here are a few other examples that result in awkward spacing just to illustrate what I'm talking about (iPhone / 375px screen width):

@Jdlrobson @Jdrewniak just to clarify: I think we should fix the first issue (assuming it isn't very costly), and maybe spend an hour or two max thinking about a clever solution for the second. It's not critical in my opinion.

Currently we are clipping the text but not indicating that it is clipped, so it reads like an incomplete sentence. Here is an example from staging:

Okay, looks like we can fix this one. I've submitted a patch.

Change 450180 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/skins/MinervaNeue@master] Correct blur position in tablet mode

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

is there anything we can do to better handle super short descriptions?
Although that comes with the downside of sometimes clipping the last word of a short description.

Personally, I don't think we should add any more hacks here. The code is already pretty brittle/magic and I'm surprised we've got this far!! Personally, I think clipping the last word of a short description is more problematic than the positioning of "learn more" given from an editors perspective the issue we are solving for them is showing these templates. If we clip a single line we would be encouraging longer edit summaries (as an editor I'd be wanting to make it 2 lines long to convey more information). I'm happy to jump into a hangout to discuss trade-offs here.

is there anything we can do to better handle super short descriptions?
Although that comes with the downside of sometimes clipping the last word of a short description.

Personally, I don't think we should add any more hacks here. The code is already pretty brittle/magic and I'm surprised we've got this far!! Personally, I think clipping the last word of a short description is more problematic than the positioning of "learn more" given from an editors perspective the issue we are solving for them is showing these templates. If we clip a single line we would be encouraging longer edit summaries (as an editor I'd be wanting to make it 2 lines long to convey more information). I'm happy to jump into a hangout to discuss trade-offs here.

This is obviously up to @alexhollender but my suggestion would be to leave as-is. I am a bit weary of getting too deep into edge cases.

I don't think there's a great way to handle super short descriptions. One thing that I was thinking of doing was increasing the right padding from 8 to 16px, which I was hoping would push an extra word down to the second line. Typographically speaking, having just one word on a line is considered bad practice, but on the web this should be considered unavoidable.

To my extra padding idea, it looks like the extra padding caused the word to hyphenate instead of being entirely placed on the second line. Not sure if this is desirable, @alexhollender ?

current paddingextra padding

note: Of course this word-break applies only to this iPhone size and this citation notice.

@Jdrewniak thanks for taking a stab at it. I'd say let's drop it and just fix the large screen + long description thing.

Jdlrobson updated the task description. (Show Details)Aug 8 2018, 5:23 PM

Change 450180 merged by jenkins-bot:
[mediawiki/skins/MinervaNeue@master] Correct blur position in tablet mode

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

Loaded on reading web staging. Over to you @alexhollender

👍 moving right along

Looking good so far

ovasileva closed this task as Resolved.Aug 21 2018, 8:36 AM

Confirming based on the screenshots. I think we're done here!

🎉🎉🎉 Excellent work, y'all!



Moving to signoff