359961 - prototype event dialog : attendee list has some usability issues

Status: RESOLVED FIXED

(Calendar :: General, defect)

Description

[Michael Büttner (no reviews TFN)]

the attendee list of the 'invite attendees'-dialog has some usability issues.
- cursor traveling does not work as expected, i.e. while entering a name it's not possible to move the cursor left or right.
- after a name has been entered the cursor should be placed on the next line automatically.
- pressing tab should always move to the next line.
- once the autoselect-box came up cursor traveling is screwed up, i.e. cursor up or down brings an empty popup instead of moving to the previous or next line.

Comment 1

[Michael Büttner (no reviews TFN)]

Attachment: patch v1

patch resolves above mentioned issues.

Comment 2

[Thomas Benisch]

I only have some minor issues.

+            var set_focus = function func() {

In this case the function name func is unnecessary.

+              if (aRow <= firstVisibleRow)
+                listbox.scrollToIndex(aRow-1);
+              else
+                if (aRow-1 >= (firstVisibleRow+numOfVisibleRows))
+                  listbox.scrollToIndex(aRow-numOfVisibleRows);

I prefer to use curly brackets.
 
-            var input = document.getAnonymousElementByAttribute(node, "anonid", "input");
-            input.focus();
+              var input = document.getAnonymousElementByAttribute(node, "anonid", "input");
+              input.focus();
+            }
+            setTimeout(set_focus,0);

Why do you need a timer at all?

+            case KeyEvent.DOM_VK_TAB:

What about supporting SHIFT TAB to move to the previous line?

r1=tbe

Comment 3

[Thomas Benisch]

Comment on attachment 245001 [details] [diff] [review]
patch v1

r1=tbe

Comment 4

[Michael Büttner (no reviews TFN)]

(In reply to comment #2)
> Why do you need a timer at all?
Since I need to defer this event in order to get this one executed after the focus-event which gets injected into the queue after my setFocus() method has been called.

> What about supporting SHIFT TAB to move to the previous line?
Good idea, I added this to the patch.

Checked in on trunk and MOZILLA_1_8_BRANCH -> FIXED