LAZR Javascript Library

Merge lp:~intellectronica/lazr-js/choiceedit-position into lp:lazr-js

  1. choiceedit-position
  2. Merge into toolchain
Proposed by Eleanor Berger
Status: Merged
Merged at revision: not available
Proposed branch: lp:~intellectronica/lazr-js/choiceedit-position
Merge into: lp:lazr-js
Diff against target: 17 lines (+5/-1)
1 file modified
src-js/lazrjs/choiceedit/choiceedit.js (+5/-1)
To merge this branch: bzr merge lp:~intellectronica/lazr-js/choiceedit-position
Reviewer Review Type Date Requested Status
Michael Nelson (community) code ui Approve
Review via email: mp+19956@code.launchpad.net
To post a comment you must log in.
Revision history for this message
Eleanor Berger (intellectronica) wrote :

This branch changes the positioning of the ChoiceEdit control so that the currently selected item is positioned directly under the mouse. This behaviour is more predictable, and we intended to implement it that way originally, but didn't do it because we were hoping that YUI will eventually provide a convenient way to do that. This never happened, and as part of my fix for bug #412925 I just went ahead and implemented it this way (with not much code anyway). The exception to the rule remains that if to position the box correctly will require drawing some of it outside of the viewable area, the position is corrected. I think that's an acceptable compromise, since drawing some of the items out of the screen will be a serious usability problem.

lp:~intellectronica/lazr-js/choiceedit-position updated
170. By Eleanor Berger

Use a selector instead of iterating over many items.

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)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'src-js/lazrjs/choiceedit/choiceedit.js'
2--- src-js/lazrjs/choiceedit/choiceedit.js 2009-11-18 21:34:09 +0000
3+++ src-js/lazrjs/choiceedit/choiceedit.js 2010-02-23 13:16:18 +0000
4@@ -521,8 +521,12 @@
5 */
6 _positionCorrectly: function(e) {
7 var boundingBox = this.get('boundingBox');
8+ var selectedListItem = boundingBox.query('span.current');
9 valueX = this._mouseX - (boundingBox.get('offsetWidth') / 2);
10- valueY = this._mouseY - (boundingBox.get('offsetHeight') / 2);
11+ valueY = (this._mouseY -
12+ this.get("headerContent").get('offsetHeight') -
13+ selectedListItem.get('offsetTop') -
14+ (selectedListItem.get('offsetHeight') / 2));
15 if (valueX < 0) {
16 valueX = 0;
17 }

Subscribers

People subscribed via source and target branches