Page MenuHomePhabricator

Page Preview prevents you from clicking link in certain situations
Closed, ResolvedPublic1 Story Points

Description

I've noticed that often times page previews appear directly beneath your cursor preventing you from being able to click the link you are hovering over. It would be nicer if the page preview was offset a few pixels from your cursor so you could still click the link after getting the page preview, or alternately make the entire page preview clickable (including the footer and pokey).

Steps To Reproduce

  • Navigate to a page, e.g. https://en.wikipedia.org/wiki/Tim_Berners-Lee.
  • Dwell on a link at least half way down the page so that the preview is above the link.
  • Observe that the preview's pokey ends up underneath the mouse cursor and isn't clickable.

Expected

The pokey should never overlap the link

Solution

See T161366#3189674.

Event Timeline

kaldari created this task.Mar 25 2017, 3:28 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 25 2017, 3:28 AM
bmansurov added a subscriber: bmansurov.

@kaldari can you please provide a link where this is happening? Also, what browser are you using.

kaldari closed this task as Resolved.Mar 30 2017, 8:57 AM
kaldari claimed this task.

This seems to be fixed now.

kaldari reopened this task as Open.Apr 7 2017, 8:06 AM

OK, just ran into this problem again. If you go to the article Tim Berners-Lee and hover over the upper half of the link "LCD" (on a small monitor/window so that the page preview appears above the link rather than below), the page preview will pop-up under your cursor and you won't be able to follow the link. The bug is basically that the footer part of the pop-up isn't clickable (while the rest of it is).

phuedx updated the task description. (Show Details)Apr 7 2017, 8:51 AM
phuedx added a subscriber: phuedx.

Thanks for the write-up @kaldari. I've condensed it and added it to the description.

kaldari updated the task description. (Show Details)Apr 8 2017, 1:19 AM
kaldari updated the task description. (Show Details)
Jdlrobson reassigned this task from kaldari to Nirzar.Apr 10 2017, 7:01 PM
Jdlrobson triaged this task as Low priority.
Jdlrobson added subscribers: Nirzar, Jdlrobson.

We talked about this and it seems the correct solution to this is for the pokey to not overlap the link by moving it up/down a few pixels. So it would appear above or below the link @Nirzar any reason not to do this?

^ I think that was the right thing to do…

@Nirzar any reason not to do this?

Seems like a bug to me. the "pokey" should not overlap the hyperlink.

Jdlrobson removed Nirzar as the assignee of this task.Apr 11 2017, 8:24 PM
Jdlrobson updated the task description. (Show Details)
Jdlrobson moved this task from Needs Analysis to Upcoming on the Readers-Web-Backlog board.

The issue here is in the createLayout function in src/renderer.js:

src/renderer.js
// Change the Y position to the top of the link
if ( event.pageY ) {
	// Since client rectangles are relative to the viewport,
	// take scroll position into account.
	offsetTop = getClosestYPosition(
		event.pageY - $window.scrollTop(),
		link.get( 0 ).getClientRects(),
		true
	) + $window.scrollTop();
}

We're not subtracting the size of the pokey from offsetTop in this situation – we are in the case where the event isn't a mouse event (event.pageY is undefined).

offsetTop = getClosestYPosition( /* ... */ ) + $window.scrollTop() - SIZES.pokeySize;

should suffice.

phuedx renamed this task from Page Preview prevents you from clicking link to Page Preview prevents you from clicking link in certain situations.Apr 18 2017, 1:20 PM
phuedx updated the task description. (Show Details)
Jdlrobson added a project: patch-welcome.
Jdlrobson added a project: good first bug.

We might want to consider this in next sprint. More we can wrap up the better.

Restricted Application added a subscriber: TerraCodes. · View Herald TranscriptApr 18 2017, 8:51 PM
MBinder_WMF set the point value for this task to 1.Apr 19 2017, 5:38 PM
phuedx claimed this task.Apr 20 2017, 3:07 PM
phuedx moved this task from To Do to Doing on the Reading-Web-Sprint-96 board.

Change 349231 had a related patch set uploaded (by Phuedx):
[mediawiki/extensions/Popups@master] Don't occlude link when preview is above mouse

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

Change 349231 merged by jenkins-bot:
[mediawiki/extensions/Popups@master] Don't occlude link when preview is above mouse

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

Jdlrobson reassigned this task from phuedx to ABorbaWMF.Apr 20 2017, 4:47 PM
Jdlrobson moved this task from Needs Code Review to Needs QA on the Reading-Web-Sprint-96 board.

I hope this is clear!
Let me know if you need better reproduction instructions!

phuedx updated the task description. (Show Details)Apr 20 2017, 5:18 PM
kaldari updated the task description. (Show Details)Apr 20 2017, 5:53 PM

^ It's yuuuuge!

Tested on the beta cluster on a variety of systems and browser. Appears to be working properly.

EddieGP moved this task from Backlog to Doing on the good first bug board.Apr 20 2017, 10:40 PM
phuedx reassigned this task from ABorbaWMF to Jdlrobson.Apr 21 2017, 9:42 AM
phuedx added a subscriber: ABorbaWMF.
phuedx closed this task as Resolved.Apr 21 2017, 12:28 PM
phuedx claimed this task.

I guess this is for me to sign off!