Closed Bug 134273 Opened 18 years ago Closed 17 years ago

Select Addresses: UI issues with focused buttons

Categories

(SeaMonkey :: MailNews: Address Book & Contacts, defect)

defect
Not set

Tracking

(Not tracked)

VERIFIED FIXED
mozilla1.4alpha

People

(Reporter: olgam, Assigned: shliang)

References

Details

(Keywords: polish, Whiteboard: [adt2 rtm],custrtm- [ue2])

Attachments

(2 files, 3 obsolete files)

Build: 03-28-2002 trunk

Overview: Select Addresses dialog: No action but closing when press Enter and a
button has focus ring.
1. New or Edit button: press Tab to move focus on it. Press Enter - dialog is
closed without bringing expected one.
2. Remove button when has dotted line around and press Enter - should remove
selected address
3. What is our expectation for default control: OK button? Pressing on Enter
closes the dialog all the time.

I am going to separate issues with To, Cc, Bcc buttons.
QA Contact: nbaca → olgam
"To" button should be default when dialog opens, not the "OK" button.

From bug 117666

----- Additional Comment #2 From jglick@netscape.com 2002-01-03 10:18 -----

I'd like to see the 4.x behavior implemented for this dialog.

Hitting Enter activates the "To" button. If user selects "Cc" button, Enter now 
activates the "Cc" button. Bcc as well. If the collection area has focus, Enter 
will activate the "OK" button.
reassigning. UI team wants this bug fixed for rtm. unexpected behavior trips up
users. 
Assignee: racham → shliang
Keywords: nsbeta1+, polish
Whiteboard: [adt2 rtm]
Whiteboard: [adt2 rtm] → [adt2 rtm],custrtm-
Attached patch patch (obsolete) — Splinter Review
the fix looks good.

one suggestion:

please follow the convention of calling global variables gFoo, instead of foo.
I can see that some of the other variables in this file that don't follow this.

Note, you don't have to clean those existing ones.  But if you do, you'll get
300 extra points.

+var toButton;
+var ccButton;
+var bccButton;
+var activatedButton;
Attached patch patch (obsolete) — Splinter Review
Attachment #85683 - Attachment is obsolete: true
Comment on attachment 85819 [details] [diff] [review]
patch

r=sspitzer

shuehan convinced me that at this point,  the reward for changing the existing
globals did not outweigh the risk, and she's right.

-300 points for me!
Attachment #85819 - Flags: review+
mscott - can you sr this one, so we can get it in soon. thanks!
Blocks: 143047
Whiteboard: [adt2 rtm],custrtm- → [adt2 rtm],custrtm- [needs r=]
Whiteboard: [adt2 rtm],custrtm- [needs r=] → [adt2 rtm],custrtm- [needs r=][ue2]
setting target milestone for buffy. 
Target Milestone: --- → mozilla1.2alpha
Blocks: 156960
QA Contact: olgam → laurel
QA Contact: laurel → nbaca
Attached patch updated patch (obsolete) — Splinter Review
Attachment #85819 - Attachment is obsolete: true
Attachment #113044 - Flags: superreview?(sspitzer)
Attachment #113044 - Flags: review?(cavin)
Comment on attachment 113044 [details] [diff] [review]
updated patch

r=cavin.
Attachment #113044 - Flags: review?(cavin) → review+
Target Milestone: mozilla1.2alpha → mozilla1.4alpha
Comment on attachment 113044 [details] [diff] [review]
updated patch

the goal is that if the abResultsTree has focus, or the QS text field, we need
to do special things
otherwise, let the event bubble up (and act like the user hit ok)

addressBucket isn't the only element that could have focus.  (ex: addressbook
picker)

So instead of adding a check for "addressBucket", let's just check for the two
elements that want to do special things ok enter, and let the rest fall
through.

how about:


+  if (event.keyCode == 13) {
+    var focusedElement = document.commandDispatcher.focusedElement;
+    if (focusedElement) {
+      if (focusedElement == gSearchInput.inputField)
+	 event.preventBubble();  // don't close the dialog on enter
+      else if (focusedElement.id == "abResultsTree") {
+	 event.preventBubble();  // don't close the dialog on enter
+	 gActivatedButton.doCommand();
+      }
+    }
+  }
Attachment #113044 - Flags: superreview?(sspitzer) → superreview-
whoops.  shuehan has given me a clue.

from jglick's comment #2:

> I'd like to see the 4.x behavior implemented for this dialog.
> Hitting Enter activates the "To" button. If user selects "Cc" button, Enter now 
> activates the "Cc" button. Bcc as well. If the collection area has focus, Enter 
> will activate the "OK" button.

let me re-review.
Status: NEW → ASSIGNED
Whiteboard: [adt2 rtm],custrtm- [needs r=][ue2] → [adt2 rtm],custrtm- [ue2]
Comment on attachment 113044 [details] [diff] [review]
updated patch

thanks for the info, shuehan.

how about this:

+  if (event.keyCode == 13) {
+    var focusedElement = document.commandDispatcher.focusedElement;
+    if (focusedElement && (focusedElement.id == "addressBucket"))
+      return;
+
+    event.preventBubble();  // enter should not close the dialog
+
+    if (focusedElement && (focusedElement.id == "abResultsTree"))
+      gActivatedButton.doCommand();
+  }
Attached patch revised patchSplinter Review
Attachment #113044 - Attachment is obsolete: true
Comment on attachment 115408 [details] [diff] [review]
revised patch

r/sr=sspitzer

thanks for the patch, and thanks for clearing up my confusion.
Attachment #115408 - Flags: superreview+
Attachment #115408 - Flags: review+
marking fixed.
Status: ASSIGNED → RESOLVED
Closed: 17 years ago
Resolution: --- → FIXED
Trunk build 2003-03-03: WinXP, Linux RH 8
Reopening due to items 2 and 3 not working as expected.

1. With "To" selected, Enter adds another entry to the collection area.
2. With "Cc" selected pressing Enter has no effect
3. With "Bcc" selected pressing Enter has no effect
4. Focus on Remove button and an item in the collection area, Enter removes the
selected entry.
5. Focus on the New button and a blank Properties dialog opens
6. Focus on the Edit button and a prefilled Properties dialog opens
7. With the focus in the collection area, pressing Enter closes the Select
Addresses dialog.
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment on attachment 116219 [details] [diff] [review]
fix mistakes in checkin

r/sr

sorry for not catching this during review.
Attachment #116219 - Flags: superreview+
Attachment #116219 - Flags: review+
additional changes checked in
Status: REOPENED → RESOLVED
Closed: 17 years ago17 years ago
Resolution: --- → FIXED
Trunk build 2003-03-07: Mac 10.1.5, WinxP
Verified Fixed.
Status: RESOLVED → VERIFIED
Product: Browser → Seamonkey
You need to log in before you can comment on or make changes to this bug.