Page MenuHomePhabricator

Hovercards: cards not visible for links below the fold OR close to browser window edges
Closed, ResolvedPublic

Description

Popups are not visible if you hover over the two linked words called 'Samurai' in the last line

When hovering over a link which is

  1. Close the fold
  2. Right Edge of window
  3. Left edge of window

The popup must always flip, so it is completely visible within viewport.
If the browser generates a folio or any element, it should not obfuscate the popup. A reader should always see the full content in the popup.

Steps to Reproduce -
Hover over the word 'Samurai' in Section 2 / Line 3 in this page
https://www.mediawiki.org/wiki/User:Jaredzimmerman_(WMF)/test/47

Observe two things
'for both cases of the blue Samurai links, the popup isn't visible'
'The browser folio is visible '

Relevant Screenshots attached

Discussion Page
See Card Disappearance
https://www.mediawiki.org/w/index.php?title=Talk:Beta_Features/Hovercards&workflow=rrc8aer3wl94rqkc


Version: unspecified
Severity: normal
URL: https://www.mediawiki.org/wiki/User:Jaredzimmerman_%28WMF%29/test/47

Attached:

Details

Reference
bz62971

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 2:54 AM
bzimport set Reference to bz62971.

When hovering over a link which is

  1. Close the fold
  2. Right Edge of window
  3. Left edge of window

When it is close to the left/right edge the popup should automatically flip. Can you please provide screenshots of the last two cases as well.

I reproduced this by resizing my window so that the word "samurai" is near the bottom of the screen. The card appears to still be actually generated and displayed, it's just that it's being displayed in a non-visible portion of the screen.

I can't reproduce this bug on the sides of the browser window; for the left and right edges, the card always seems to place itself somewhere sensible.

Agree, We just need to address the issue for 'link is too close to the fold' for now.

I can think of two possible solutions:

  1. Flip the popup–show it above the link
  2. Auto-scroll the page (ewww)

Tempting as it is, Autoscroll cannot be our option, we are better than that. Its jarring for the reader and significantly changes read behavior.

So (1) from comment #4? Any other ideas?

Kipod added a comment.Apr 2 2014, 2:08 PM

(In reply to Prateek Saxena from comment #4)

I can think of two possible solutions:

  1. Flip the popup–show it above the link
  2. Auto-scroll the page (ewww)

autoscrolling is of course not an option. hover action can't change the display of the page (with possible exception change to the element above which you are hovering).

why not take a look at how the jquery plugin "tipsy" (already included with mediawiki) does it?

it has "gravity" setting to determine if the popup should be above, below, left or right of the element (they use n,s,e,w plus combinations like ne).

the interesting option and logic there is "automatic" gravity which allows you to say "popup below element, except at the bottom portion of the window, where it's above", and similarly when location is too close to left or right edge.

as far as i remember, the "auto" options in tipsy costs maybe 2 or 4 JS lines.

peace.

Created attachment 15059
Proposed Solution for cards with images

If the link is close to the fold, the popup can horizontally flip.

Attached:

Created attachment 15060
Proposed Solution for cards without images

Popups can horizontally flip.
We can define a margin if the link are on the corner of the screen, although its dependent on the way the code has been set up.

Attached:

Prateek, I discussed some alternate solutions with Jared.
We think that the proposals attached to this bug are better suited to solving the problem. I'm assigning this bug to you so you can comment on complexity and perhaps think about the patch?

Change 130585 had a related patch set uploaded by Prtksxna:
Flip popups on X and Y axis so that they don't render below the fold

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

(In reply to Gerrit Notification Bot from comment #11)

Change 130585 had a related patch set uploaded by Prtksxna:
Flip popups on X and Y axis so that they don't render below the fold
https://gerrit.wikimedia.org/r/130585

2 comments:
// Y Flip
if ( event.clientY > ( $( window ).width() / 2 ) ) {
flippedY = true;
}

clearly, this is an error: we should look at $window.height(), not width.

2nd comment is stylistic: in JS, writing "if (condition) { boolvar = true; } is both ugly and redundant. write "boolvar = condition" instead.

so the above 3 lines shrink to:

flippedY = clientTop > $( window ).height() / 2;

more concise, simpler and clearer (and, of course, more correct, by virtue of using "height" instead of "width"...)

also, this:
if ( flippedY ) {
popup.css( {

		top: popup.offset().top - ( popup.outerHeight() + 50 )

} );
}

i found that more elegant way to do "flipY" is to set the bottom of the element instead of the top. this way, you don't have to worry about the element's height - e.g., in case the "outer height" at the point you calculate it changes later. basically in article.getOffset(), instead of
return {
top: offsetTop + 'px',
left: offsetLeft + 'px'
};
do
offsetTop -= (flippedY ? 50 : 0);
return {
(flippedY ? "bottom" : "top") : offsetTop + 'px',
left: offsetLeft + 'px'
};

and remove the piece i quoted above.

if you use the same logic of setting "right" instead of "left" in case of flippedX, the code will become significantly simpler.

peace.

(In reply to kipod from comment #12)

clearly, this is an error: we should look at $window.height(), not width.

Fixed.

more concise, simpler and clearer (and, of course, more correct, by virtue
of using "height" instead of "width"...)

Agreed on use of height, style guide to be followed - https://www.mediawiki.org/wiki/CC/JS

in case the "outer height" at the point you calculate it changes later

I get the height after its rendered so it can't change

if you use the same logic of setting "right" instead of "left" in case of
flippedX, the code will become significantly simpler.

Hmm. It *might* not be that simple. With calculations to adjust the size of the triangle and to acutally position it above or below the link, there will be enough calculations there as well.

Thanks for the review kipod! Could you please review and/or +1 the patch on Gerrit too?

Kipod added a comment.May 1 2014, 3:12 PM

(In reply to Prateek Saxena from comment #13)

Thanks for the review kipod! Could you please review and/or +1 the patch on
Gerrit too?

unfortunately, though i have a gerrit account, i did not yet learn to actually use it. tried a couple of times, but did not persist. i am probably doing something wrong, and haven't been able to review or +1 using gerrit yet.

as to your comment about style guide: i did not see there anything that says that

boolvar = <boolean expression> ;

is bad, and we should instead use

if (<boolean expression>) {
    boolvar = true;
}

if style guild indeed says that (and i doubt it), then it's a bug in the guide.

as to using "bottom" and "right" instead of "top" and "left" when appropriate: this is something i learned when i developed [[Module:Chart]] for drawing pie charts.
it uses a dirty css trick with border bevels, and requires absolutely precise placement of one corner of the elements, where the corner can be ne, nw, se or sw.
i could not make this work correctly with enough precision for any corner other than nw, by calculating "top" and "left", even though i had the exact dimensions of the elemnt.
when i switched to setting "bottom" or "right" in those cases, it works precisely as prescribed. i think browsers take some liberties with the dimensions of the elements, so when you query it in the script to calculate top and left, you may get somewhat different answers than the actual dimensions turn out to be. by setting "bottom" and/or "right" when appropriate, you avoid those problems.

peace.

(In reply to kipod from comment #14)

unfortunately, though i have a gerrit account, i did not yet learn to
actually use it. tried a couple of times, but did not persist. i am probably
doing something wrong, and haven't been able to review or +1 using gerrit
yet.

kipod: If you're logged in (username in upper right corner in RTL languages), https://gerrit.wikimedia.org/r/#/c/130585/ should offer a "Review" button where you can set +2 to -2 under "Code-Review".
Also see https://www.mediawiki.org/wiki/Gerrit/Tutorial#How_we_review_code

Change 130585 merged by jenkins-bot:
render.article: Flip popups on X and Y axis so that they don't render outside the viewport

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

Kipod added a comment.May 9 2014, 6:29 PM

(In reply to Gerrit Notification Bot from comment #16)

Change 130585 merged by jenkins-bot:
render.article: Flip popups on X and Y axis so that they don't render
outside the viewport
https://gerrit.wikimedia.org/r/130585

this patch still uses $( window ).width() instead of height() to calculate flippedY. (file resources/ext.popups.renderer.article.js, line 294)

peace.

(In reply to kipod from comment #17)

Sorry for the confusion. I accidentally fixed this in another unrelated commit. Its using .height() in the final version. Here is the link to the commit that fixes it - https://gerrit.wikimedia.org/r/#/c/130629/3/resources/ext.popups.renderer.article.js

Kipod added a comment.May 22 2014, 9:59 PM

please see bug 65433 for a demonstration of the effect i was referring to: calculating "top" can be slightly off. when flippedY is true, it is much better to use "bottom".

peace.

Prtksxna raised the priority of this task from Medium to Needs Triage.Dec 3 2014, 5:29 AM
Prtksxna moved this task from Backlog to Archive on the Page-Previews board.