Closed
Bug 953658
Opened 10 years ago
Closed 10 years ago
Groups not collapsable/expandable by keyboard
Categories
(Instantbird Graveyard :: Contacts window, defect)
Instantbird Graveyard
Contacts window
Tracking
(Not tracked)
RESOLVED
FIXED
0.2b1
People
(Reporter: benediktp, Assigned: romain)
Details
Attachments
(1 file, 2 obsolete files)
1.48 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 212 at 2009-08-12 12:18:00 UTC *** The groups can't be collapsed/expanded using the keyboard only as it seems. I couldn't find a handler in the code, so I think it's not possible at all right now -> bug.
Assignee | ||
Updated•10 years ago
|
OS: Windows XP → All
Hardware: x86 → All
Comment 1•10 years ago
|
||
*** Original post on bio 212 as attmnt 199 by pvdeejay AT gmail.com at 2009-08-17 18:05:00 UTC *** The functionality is implemented by this patch. Other enhancements can be made, that assistive technologies get an idea on the state of the group item and level an item is nested in but it's beyond this bug I believe.
Comment 2•10 years ago
|
||
*** Original post on bio 212 as attmnt 200 by pvdeejay AT gmail.com at 2009-08-18 04:42:00 UTC *** I haven't mentioned the keys used to control group expanding / collapsing buddy list groups. Originally I have bound left and right arrow keys. Later on we have further discussed ability to use numpad plus and numpad minus in the IRC channel. I have also added enter key. Numpad keys don't work on my system so it needs to be tested.
Comment 3•10 years ago
|
||
Comment on attachment 8351943 [details] [diff] [review] patch1 *** Original change on bio 212 attmnt 199 by pvdeejay AT gmail.com at 2009-08-18 04:42:51 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351943 -
Attachment is obsolete: true
Reporter | ||
Comment 4•10 years ago
|
||
*** Original post on bio 212 at 2009-08-18 10:10:57 UTC *** I changed your code as follows. The switch/case wasn't practical since + and - are treated as normal characters and their value is not stored in aEvent.keyCode but aEvent.charCode. By the way it doesn't distinguish between normal + or - keys and keypad. Maybe someone could create a diff off it (The diffs I create here on Windows are strange..) // Handle key pressing keyPress: function bl_keyPress(aEvent) { var item = document.getElementById("buddylistbox").selectedItem; if ( aEvent.keyCode == aEvent.DOM_VK_RETURN || aEvent.keyCode == aEvent.DOM_VK_ENTER ){ // If Enter or Return is pressed, open a new conversation if ( item.localName == "buddy" ) item.openConversation(); else item.close(); } // 45 is charCode for - else if ( aEvent.keyCode == aEvent.DOM_VK_LEFT || aEvent.charCode == 45 ){ if ( item.localName == "group" && !item.hasAttribute("closed") ) item.close(); } // 43 is charCode for + else if ( aEvent.keyCode == aEvent.DOM_VK_RIGHT || aEvent.charCode == 43 ){ if ( item.localName == "group" && item.hasAttribute("closed") ) item.close(); } return; },
Reporter | ||
Comment 5•10 years ago
|
||
*** Original post on bio 212 at 2009-08-18 10:39:35 UTC *** Just noticed that I missed the check for the DOM_VK values, just in case that it works on any platform. // Handle key pressing keyPress: function bl_keyPress(aEvent) { var item = document.getElementById("buddylistbox").selectedItem; if ( aEvent.keyCode == aEvent.DOM_VK_RETURN || aEvent.keyCode == aEvent.DOM_VK_ENTER ){ // If Enter or Return is pressed, open a new conversation if ( item.localName == "buddy" ) item.openConversation(); else item.close(); } // 45 is charCode for - else if ( aEvent.keyCode == aEvent.DOM_VK_LEFT || aEvent.keyCode == aEvent.DOM_VK_SUBTRACT || aEvent.charCode == 45 ){ if ( item.localName == "group" && !item.hasAttribute("closed") ) item.close(); } // 43 is charCode for + else if ( aEvent.keyCode == aEvent.DOM_VK_RIGHT || aEvent.keyCode == aEvent.DOM_VK_ADD || aEvent.charCode == 43 ){ if ( item.localName == "group" && item.hasAttribute("closed") ) item.close(); } return; },
Assignee | ||
Comment 6•10 years ago
|
||
*** Original post on bio 212 at 2009-08-18 11:27:17 UTC *** Given http://mxr.mozilla.org/mozilla-central/source/toolkit/content/widgets/tree.xml#709 , I think we should drop out any + or - support. Moreover VK_ADD and VK_SUBTRACT doesn't work on Linux either, so it seems that we don't have a way to distinguish between numpad_+ and + I prefer switch case implementation here, it is more readable (with an additional line break after "break;" instructions).
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 212 as attmnt 202 at 2009-08-18 22:36:00 UTC *** Adding extra line breaks, removing VK_ADD and VK_SUBTRACT, and adding a test on localName when the key is ENTER or RETURN (in case we implement other things that buddy and group in the future).
Attachment #8351946 -
Flags: review?(florian)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8351944 [details] [diff] [review] patch2 *** Original change on bio 212 attmnt 200 at 2009-08-18 22:36:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8351944 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → bugzilla
Status: NEW → ASSIGNED
Comment 9•10 years ago
|
||
Comment on attachment 8351946 [details] [diff] [review] proposed patch *** Original change on bio 212 attmnt 202 at 2009-08-21 10:39:58 UTC *** This looks good. By the way, why do we have a 'return;' at the end of the method?
Attachment #8351946 -
Flags: review?(florian) → review+
Comment 10•10 years ago
|
||
*** Original post on bio 212 at 2009-08-21 21:19:23 UTC *** I edited the patch a bit before pushing it so that the method doesn't cause JavaScript errors when there's no selected item (for example because the buddy list is empty). Pushed https://hg.instantbird.org/instantbird/rev/58f060743fb2
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.2b1
Comment 11•10 years ago
|
||
*** This had the wrong assignee on BIO and failed to properly import into BMO. Reassigning. ***
Assignee: bugzilla → romain
You need to log in
before you can comment on or make changes to this bug.
Description
•