Last modified: 2014-05-22 21:59:24 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T64971, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 62971 - Hovercards: cards not visible for links below the fold OR close to browser window edges
Hovercards: cards not visible for links below the fold OR close to browser wi...
Status: RESOLVED FIXED
Product: MediaWiki extensions
Classification: Unclassified
Popups (Other open bugs)
unspecified
All All
: Normal normal (vote)
: ---
Assigned To: Prateek Saxena
https://www.mediawiki.org/wiki/User:J...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2014-03-23 06:37 UTC by Vibha Bamba
Modified: 2014-05-22 21:59 UTC (History)
6 users (show)

See Also:
Web browser: Google Chrome
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Popups are not visible if you hover over the two linked words called 'Samurai' in the last line (88.58 KB, image/png)
2014-03-23 06:37 UTC, Vibha Bamba
Details
Proposed Solution for cards with images (569.96 KB, image/png)
2014-04-09 00:37 UTC, Vibha Bamba
Details
Proposed Solution for cards without images (572.44 KB, image/png)
2014-04-09 00:38 UTC, Vibha Bamba
Details

Description Vibha Bamba 2014-03-23 06:37:17 UTC
Created attachment 14881 [details]
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
Comment 1 Prateek Saxena 2014-03-23 06:39:24 UTC
> 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.
Comment 2 Dan Garry 2014-03-24 20:51:11 UTC
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.
Comment 3 Vibha Bamba 2014-03-24 20:54:36 UTC
Agree, We just need to address the issue for 'link is too close to the fold' for now.
Comment 4 Prateek Saxena 2014-03-25 01:21:19 UTC
I can think of two possible solutions:
 1. Flip the popup–show it above the link
 2. Auto-scroll the page (ewww)
Comment 5 Vibha Bamba 2014-03-25 17:18:51 UTC
Tempting as it is, Autoscroll cannot be our option, we are better than that. Its jarring for the reader and significantly changes read behavior.
Comment 6 Prateek Saxena 2014-03-27 13:55:38 UTC
So (1) from comment #4? Any other ideas?
Comment 7 kipod 2014-04-02 14:08:24 UTC
(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.
Comment 8 Vibha Bamba 2014-04-09 00:37:24 UTC
Created attachment 15059 [details]
Proposed Solution for cards with images

If the link is close to the fold, the popup can horizontally flip.
Comment 9 Vibha Bamba 2014-04-09 00:38:31 UTC
Created attachment 15060 [details]
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.
Comment 10 Vibha Bamba 2014-04-14 22:25:09 UTC
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?
Comment 11 Gerrit Notification Bot 2014-04-30 14:16:12 UTC
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
Comment 12 kipod 2014-04-30 23:37:57 UTC
(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.
Comment 13 Prateek Saxena 2014-05-01 03:24:46 UTC
(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?
Comment 14 kipod 2014-05-01 15:12:25 UTC
(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.
Comment 15 Andre Klapper 2014-05-01 21:08:34 UTC
(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
Comment 16 Gerrit Notification Bot 2014-05-09 13:53:03 UTC
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
Comment 17 kipod 2014-05-09 18:29:18 UTC
(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.
Comment 18 Prateek Saxena 2014-05-09 19:50:03 UTC
(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
Comment 19 kipod 2014-05-22 21:59:24 UTC
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.

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links