Code review comment for lp:~intellectronica/lazr-js/choiceedit-position

Revision history for this message
Michael Nelson (michael.nelson) wrote :

That's an excellent improvement for lots of situations Tom. I can't see any way to handle the situation where the box would be outside the viewable area either (other than moving the user's cursor ;)).

One possibility: if we always presented the current item at the top of the widget, with the available options below, we could guarantee that both the current item was under the mouse and the other options were visible (except at the bottom of the page where we'd hit the same issue, but I think that situation would be much less frequent). Thoughts?

14:10 < noodles775> intellectronica: code-wise, any reason for not using boundingBox.query('span.current') instead of iterating through all the spans?
14:10 < intellectronica> noodles775: no, i just didn't think of that. let me give that a try.
14:12 < intellectronica> noodles775: yeah, that works fine. changing.
14:13 < noodles775> Great, r=me.
14:13 < intellectronica> noodles775: thanks!

review: Approve (code ui)

« Back to merge proposal