Page MenuHomePhabricator

Entire page preview should be clickable
Closed, DeclinedPublic

Description

Update [08/08/18]
The original fix for this patch caused another regression (T200940) - by making it impossible to ctrl+command+click links into new tabs. The patch was reverted, this task reopened with updated acceptance criteria


From T192710: [Feedback] Personal observations about the page previews feature usability

  • I feel the area that is clickable on the panel is strange- I guess it is only on the part that is 100% visible, and that a gap has to be added between the preferences and the button, but maybe the gap can be just a corner on the right-down, or at least extend the clickable parts a bit more towards the bottom part? Image attached:
    clickable_area.png (861×1 px, 231 KB)

Problem

For readers it is unclear where the clickable area on the page previews panel begins. Intuitively one might think that the content (image + text) is clickable, but currently the bottom portion of the text content is not clickable.

When fixing this, we'll also need to make sure that we don't break default browser behaviour, for example, opening a link in a new tab via keyboard shortcuts (command+ctrl+click)

Design notes

  • the entire panel, aside from the cog+cog hover area, should click-through to the respective page
  • the entire panel should have pointer:cursor

Artboard Copy.jpg (297×710 px, 160 KB)

Developer notes

Currently the links only apply to the thumbnail portion and the extra portion via links inside the preview itself (note the red line surrounding the link inside this preview:)

Screen Shot 2018-04-25 at 11.34.19 AM.png (332×592 px, 175 KB)

Proposed solution

We need to propose a solution.

Bind a click to anywhere on the preview to the link via JS. The settings cog should stop propagation to the parent element. Note, there is a danger of accessibility problems here but these are recorded and will be dealt with later. [Update: doesn't work. Breaks open in new tab]

QA

  • Check clicks on the footer (to left of settings cog)
  • It should be possible to open a tab by middle clicking mouse
  • It should be possible to open a tab by control clicking

Related Objects

Event Timeline

There are a very large number of changes, so older changes are hidden. Show Older Changes
alexhollender_WMF updated the task description. (Show Details)

@Jdlrobson can you take a look at this when you have a second? I'd love to move it up in the queue if it doesn't seem to complicated

@alexhollender added some thoughts. Seems a bit of risk here. Answer the questions and throw it into needs analysis in the backlog so it enters the developer queue :)

@Jdlrobson just put answers to your questions in a Design notes section

Let's attempt to estimate this with the other developers and see what comes back.

It's also a little tricky to select the text since the cursor isn't as an I-beam.

Screenshot from 2018-05-09 08-43-23.png (685×886 px, 350 KB)

@Niedzielski ah that's a good point. I guess in this case clicking is more common than selecting text. Maybe we can't have our cake and eat it too? Will try to think of a solution here, let me know if you've got any ideas.

@Jdlrobson I think this is good to go? Design notes in the description describe what the UX should be.

@alexhollender it's not clear how this impacts accessibility and which option to go with.
In particular, it's not clear whether the area around the settings cog is clickable or not.
I think what would help is a red polygon detailing the clickable region (similar to developer notes but showing what we'd like to get to). Can that be arranged?

@Jdlrobson let me know if this clarifies. Regarding accessibility: I did speak to Volker about this. No conclusion thus far, but I think we can keep that a separate topic (tracked here T195718).

Artboard Copy.jpg (297×710 px, 160 KB)

Jdlrobson renamed this task from Clickable area on the panel is strange to Entire page preview should be clickable.Jun 19 2018, 5:02 PM
Jdlrobson updated the task description. (Show Details)

The footer element is blocking the lower part of the popup. Is there any reason this can't be fixed just by fiddling with the CSS so that its width doesn't stretch across the whole element, and repositioning things so that this doesn't affect the appearance?

Jdlrobson set the point value for this task to 3.Jun 26 2018, 4:23 PM

Change 442880 had a related patch set uploaded (by Jdlrobson; owner: Jdlrobson):
[mediawiki/extensions/Popups@master] Whole popup area should be clickable

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

Change 442880 merged by Jdlrobson:
[mediawiki/extensions/Popups@master] Whole popup area should be clickable

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

Tested this quite a bit yesterday. Looks good to me on staging. Tested on Windows and Mac and Chrome, Firefox, Safari, IE and Edge.

ovasileva added a subscriber: ovasileva.

Looks good!

Jdlrobson removed the point value for this task.
Jdlrobson lowered the priority of this task from Medium to Low.
Jdlrobson added a subscriber: ABorbaWMF.

Marking as low for the time being. Correct me if I'm wrong!

Marking as low for the time being. Correct me if I'm wrong!

Sounds good to me.

@Jdlrobson @ovasileva why low priority? Thinking that since page previews are front & center this bug is pretty visible

Patch has been reverted. We'll need to rethink the approach here at a later date.

What ended up happening with this patch?

How would y'all feel about bringing it onto the sprint board? I feel like the priority should be higher than low.

@alexhollender not sure I understand. The original fix here caused T200940 so we reverted it. We don't currently know a way to fix this without a big risky rewrite of the page previews render code.

We'd need to schedule some analysis (spike?) if we want to pursue a solution to this problem.
My preference is to wait till we have less projects in flight to do that.

@alexhollender not sure I understand. The original fix here caused T200940 so we reverted it. We don't currently know a way to fix this without a big risky rewrite of the page previews render code.

We'd need to schedule some analysis (spike?) if we want to pursue a solution to this problem.
My preference is to wait till we have less projects in flight to do that.

+ 1 on this. We'd want to fix this eventually but as it seems fairly complex and we have higher-priority project in-flight, I'd wait and reconsider a bit further down the line. We can bump up the priority then.

LGoto added a subscriber: LGoto.

This task was closed as part of backlog upkeep. If you believe it was closed in error, please respond on the ticket.