Opened 10 years ago

Closed 9 years ago

Last modified 9 years ago

#9945 closed task (fixed)

Migrate widgets to use _HasDropDown

Reported by: Nathan Toone Owned by: bill
Priority: high Milestone: 1.6
Component: Dijit Version: 1.3.2
Keywords: Cc:
Blocked By: Blocking:

Description (last modified by bill)

The following widgets should be migrated to use the common _HasDropDown code:

  1. dijit.form.ComboBox
  2. dijit.form.FilteringSelect (extends from ComboBox)
  3. dijit.form._DateTimeTextBox
  4. dijit.form.DateTextBox (extends from _DateTimeTextBox)
  5. dijit.form.TimeTextBox (extends from _DateTimeTextBox)

Attachments (1)

hdd.patch (35.7 KB) - added by bill 9 years ago.
patch (for sake keeping); haven't resolved when to call evt.preventDefault() on the SPACE key (yes for Button, Select, no for *Box)

Download all attachments as: .zip

Change History (25)

comment:1 Changed 10 years ago by bill

Component: DojoX FormDijit

Just marking as dijit so it shows up in my reports.

comment:2 Changed 10 years ago by bill

Description: modified (diff)

comment:3 Changed 9 years ago by Nathan Toone

Owner: Nathan Toone deleted

Unassigning my tickets.

comment:4 Changed 9 years ago by bill

Milestone: future1.6
Owner: set to bill
Status: newassigned

comment:5 Changed 9 years ago by bill

Description: modified (diff)

(In [22555]) Convert _DateTimeTextBox to extend _HasDropDown, so it gets an icon (down arrow button) to open the drop down. The icon's display is controlled by a hasDownArrow parameter, same as ComboBox.

Tabbing into the widget no longer causes the drop down to (automatically) open. Clicking the field causes the drop to open, although that behavior can be disabled by setting the new "openOnClick" parameter to false. Of course, clicking the down arrow button (if it's displayed) will always open the drop down.

Changed _HasDropDown to not expect the drop down widget (this.dropDown) to exist initially, prior to the user clicking the down arrow, as DateTextBox creates it on demand. Also fixed the connections for the key handlers, which should clearly be on this.focusNode rather than this._buttonNode.

Had to modify _TimePicker some because the handleKey() callback from _HasDropDown is being called on keyup rather than keypress like before, so evt.charOrCode isn't set. Maybe _HasDropDown should be monitoring keypress rather than keyup.

Made a new DropDownBox.html template used for DateTextBox/TimeTextBox, and expected to be used for ComboBox/FilteringSelect once I get those converted to extend _HasDropDown.

Open issues include:

  • confirm aria roles are correct
  • mouseover of the fields in the _TimePicker no longer makes them expand in width
  • TimeTextBox needs more tests

Fixes #5151, refs #9945 !strict.

comment:6 Changed 9 years ago by bill

(In [22558]) Make ENTER/SPACE keys close TimeTextBox drop down and select value, refs #9945 !strict. But still wondering if ENTER/SPACE should be handled in DateTimeTextBox itself.

comment:7 Changed 9 years ago by bill

(In [22604]) Switch _HasDropDown to monitor and forward keypress events, rather than keyup events, so key repeat works and we can use evt.charOrCode like before. Reverting charOrCode --> keyCode changes from [22555] that are no longer needed. Refs #9945 !strict.

Changed 9 years ago by bill

Attachment: hdd.patch added

patch (for sake keeping); haven't resolved when to call evt.preventDefault() on the SPACE key (yes for Button, Select, no for *Box)

comment:8 Changed 9 years ago by bill

(In [22619]) Need stopEvent() after processing keystrokes so that (in particular) ESC on a DateTextBox in a Dialog will just close the DateTextBox's drop down, not the whole dialog. Refs #9945.

comment:9 Changed 9 years ago by bill

(In [22620]) Test updates related to DateTextBox _HasDropDown conversion, refs #9945.

comment:10 Changed 9 years ago by bill

(In [22621]) Fix space key for opening drop downs. Followup from [22604]: keypress uses charOrCode not keyCode. Refs #9945 !strict.

comment:11 Changed 9 years ago by bill

(In [22658]) Fix bug introduced in DateTextBox --> _HasDropDown conversion where SPACE character is no longer accepted as direct keyboard input into the box. (It needs to be accepted since "September 9, 1999" is a valid date format.)

I fixed this by removing the code from _HasDropDown to prevent IE screen jump on space/enter/down arrow keys. I can't reproduce that bug anymore, even as far back as dojo 1.1, nor can I find a ticket about it. If it turns out we need the code after all, then have to make it conditional so it runs for <span>/<div>/<input type=button>, but not for <input type=text> (or type=password).

As a side note, with the hasDownArrow=false setting, the SPACE character will open the drop down rather than appearing as input, so it can't (currently) be used in conjunction with long format dates.

Refs #9945 !strict.

comment:12 Changed 9 years ago by bill

(In [22659]) While DateTextBox drop down is open, arrow keys (etc.) move position in the drop down but shouldn't affect caret position in the <input>.

Refs #9945 !strict.

comment:13 Changed 9 years ago by bill

Resolution: fixed
Status: assignedclosed

(In [22660]) Convert ComboBox to extend _HasDropDown. The migration involved:

  • make ComboBox use the code in _HasDropDown to open/close/size the drop down, and the code to monitor for clicks/keypresses to open/close drop down
  • a lot of renaming: this._popupWidget --> this.dropDown, this.downArrowNode --> this._buttonNode, this.comboNode --> this.domNode, this._isShowingNow --> this._opened, _onKeyPress --> _onKey, _hideResultList --> closeDropDown
  • removed "state" from _HasDropDown since it conflicted with ValidationTextBox.state and since it had the same meaning as this._opened
  • added maxHeight: -1 setting to _HasDropDown to cap the drop down size to what will fit in the viewport

Since ComboBox's drop down opens asynchronously this was a tricky change, because _HasDropDown wasn't really designed for the drop down to open asynchronously, except for the first time it's shown, when it's loading from an href.

Fixes #9945, #11461 !strict. Refs #10229 but doesn't fix completely.

comment:14 Changed 9 years ago by liucougar

(In [22664]) refs #9945: don't stop a space key press event in a popup if it happens in a text input element

comment:15 Changed 9 years ago by bill

(In [22689]) Fixes so FilteringSelect isn't marked invalid just by clicking the down arrow icon, refs #9945 !strict. There's still an issue where typing into the FilteringSelect marks it invalid even though the drop down shows some valid completions.

comment:16 Changed 9 years ago by Douglas Hays

Resolution: fixed
Status: closedreopened

After [22604], TimeTextBox? can no longer filter times by typing the hour. Refs #11485. In test_TimeTextBox.html, 4th box, you have to type "133" to get it to only show times starting with "13". Everything seems 1 character off.

comment:17 Changed 9 years ago by bill

Good catch. I made TimeTextBox._onKey() [re]open the drop down based on the new contents of the <input>, but _onKey() executes before the keystroke is reflected into the <input> (ie, the browser default handling of the keypress event).

comment:18 Changed 9 years ago by bill

Resolution: fixed
Status: reopenedclosed

(In [22736]) Fix filtering of TimeTextBox drop down; it wasn't acknowledging the last character typed. Fixes #9945 !strict.

comment:19 Changed 9 years ago by Douglas Hays

(In [22757]) Refs #9945. [22660] removed combobox.downArrowNode but the test still used it.

comment:20 Changed 9 years ago by bill

(In [22762]) [22604] (refs #9945) broke opening a DropDownButton via the space key on IE. Root problem was that the ondijitclick code supresses onkeypress events for the SPACE key (on IE). Also refs #10253. !strict

comment:21 Changed 9 years ago by bill

(In [22766]) Second attempt to make _HasDropDown on ondijitclick work together, fixing both DropDownButton and ComboButton on IE, refs #9945 !strict.

comment:22 Changed 9 years ago by bill

(In [22852]) [22660] accidentally removed the dojo.connect() for the compositionend handler, for triggering drop down filtering upon entering Japanese/Chinese? characters. Turns out though that code is no longer needed, FF must have fixed their bug. Refs #9945 !strict.

comment:23 Changed 9 years ago by bill

(In [24141]) For the sake of backwards compatibility, make onClick() fire for clicks on down arrow, and let that click event propagate (i.e. don't call dojo.stopEvent()). In 2.0 though I don't want to spend bytes supporting esoteric events like ComboBox.onClick at all. Fixes #12520, refs #9945 !strict on 1.6/ branch.

comment:24 Changed 9 years ago by bill

(In [24143]) For the sake of backwards compatibility, make onClick() fire for clicks on down arrow, and let that click event propagate (i.e. don't call dojo.stopEvent()). In 2.0 though I don't want to spend bytes supporting esoteric events like ComboBox.onClick at all. Fixes #12520, refs #9945 !strict on trunk.

Note: See TracTickets for help on using tickets.