Merge lp:~deryck/launchpad/pop-up-help-positioning-574682 into lp:launchpad

[Deryck Hodge]

Fix positioning of pop-up help overlays.

This fixes bug 574682. It makes pop-up help overlays consider the viewport position as part of the setElementPosition call. Currently on Launchpad, pop-up help is positioned off screen if you scroll down the page and then select the help icon (on tags, for example).

I have no idea how to test this since it's MochiKit-based. I'm happy to learn and write a test if we have anything for MochiKit-based code.

Cheers,
deryck

[Eleanor Berger (intellectronica)]

Thanks for fixing this annoying bug. I don't think it's necessary to add a test (you could use Windmill if you really wanted), but I wonder if any consideration has been given to converting this to use LAZR-JS. I very vaguely remember Maris talking about it at some point. Maybe it's worth letting him know that we're now fixing bugs in this legacy code.

The fix looks fine and works well. You can remove the if statement. It is not necessary and I don't think aids readability. Up to you.

review: Approve (code ui)

[Deryck Hodge (deryck)]

Hi, Tom.

Thanks for the review.

I looked into converting to lazr-js myself, but I didn't have time to invest when a simple fix will work. There are still other, deeper dependencies on MochiKit than this bit of code, so it didn't seem worth it to me for now. I don't think it's worth a Windmill test, either, to be honest.

I did remove the if statement. I thought it was a bit more clear why this was needed with the if statement, but no harm without either, so I dropped it.

Thanks again for the review today.

Cheers,
deryck

=== modified file 'lib/lp/services/inlinehelp/javascript/inlinehelp.js'
--- lib/lp/services/inlinehelp/javascript/inlinehelp.js	2010-01-12 16:43:00 +0000
+++ lib/lp/services/inlinehelp/javascript/inlinehelp.js	2010-05-10 22:24:27 +0000
@@ -99,6 +99,8 @@
     var help_pane_dim = elementDimensions('help-pane');
     var pos_x = Math.round(viewport_dim.w / 2) - (help_pane_dim.w / 2);
     var pos_y = Math.round(viewport_dim.h / 2) - (help_pane_dim.h / 2);
+    var viewport_pos = getViewportPosition();
+    pos_y += viewport_pos.y;
     setElementPosition(help_pane, new Coordinates(pos_x, pos_y));
     makeVisible(help_pane);