https://bugzilla.wikimedia.org/show_bug.cgi?id=62971
--- Comment #12 from kipod <[email protected]> --- (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. -- You are receiving this mail because: You are on the CC list for the bug. _______________________________________________ Wikibugs-l mailing list [email protected] https://lists.wikimedia.org/mailman/listinfo/wikibugs-l
