User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050219 Firefox/1.0+ Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.8b2) Gecko/20050219 Firefox/1.0+ The XForms input control should determine the datatype of the field that it is bound, and dynamically display the appropriate datetimepicker - either date, time or date/time. We'll also have to address Month/Year, Month, Year as well (either build into the datetimepicker control or maybe use the localized strings pulled from the datetimepicker's dateFormat.properties file to populate a select list?). Reproducible: Always

what I currently have. Works for xsd:date only, all strings are gotten from the user's locale (well, other than the choose date string on the button :), is pretty accessible (you can use the keyboard to navigate the dates, but you have to tab around to get back at the back/fwd buttons). for gmonth/gday, we can easily use a select1. for gmonthyear, spinbutton probably as well. time would be trickier, probably need spinbuttons. datetime would be both date and time in one widget

I ended up coming up with my own dropdown marker code, since the select1 one was pretty complex. I ended up using html:button with an html:img, which works pretty fine.

Review comments welcome, this can be tested.

supports date, gMonth and gDay.

Comment on attachment 201044 [details] [diff] [review] patch - date, month, year >+ <html:input type="button" class="-moz-date-back-button" >+ onclick="this.parentNode.parentNode.parentNode.parentNode.parentNode.parentNode.goBack();"/> indentation (extra space) >+ </html:td> >+ <html:td colspan="5" align="center"> >+ <html:span anonid="date"/> >+ </html:td> >+ <html:td colspan="1"> >+ <html:input type="button" class="-moz-date-fwd-button" >+ onclick="this.parentNode.parentNode.parentNode.parentNode.parentNode.parentNode.goForward();"/> indentation (extra space) >+ <field name="_uibuilt">false</field> >+ <field name="_isPickerVisibile">false</field> Spelling >+ >+input[mozType|type="http://www.w3.org/2001/XMLSchema#date"] html|td.prevMonth, >+input[mozType|type="http://www.w3.org/2001/XMLSchema#date"] html|td.nextMonth { >+ color: lightgrey; Is the color right? Maybe it should be GrayText (http://www.w3.org/TR/CSS21/ui.html#system-colors) >+} >+ >+input[mozType|type="http://www.w3.org/2001/XMLSchema#date"] html|td.prevMonth:hover, >+input[mozType|type="http://www.w3.org/2001/XMLSchema#date"] html|td.nextMonth:hover { >+ background-color: grey; I'm not sure about this color. Perhaps it is ok. >+ >+input[mozType|type="http://www.w3.org/2001/XMLSchema#date"] html|td.currentMonth:hover { >+ background-color: blue; >+ color: white; color: HighlightText; background-color: Highlight; >+} >+ >+html|input.-moz-date-back-button { >+ width: 20px; >+ background-image: url("chrome://global/skin/arrow/arrow-lft.gif") !important; >+ background-repeat: no-repeat !important; >+ background-position: center !important; >+} >+ >+html|input.-moz-date-fwd-button { >+ width: 20px; >+ background-image: url("chrome://global/skin/arrow/arrow-rit.gif") !important; >+ background-repeat: no-repeat !important; >+ background-position: center !important; >+} >+ I guess (didn't test) these buttons don't look right in WinXP default theme. Would it be possible to fix the problem? >+html|span.-moz-date-container { >+ margin: 0px; >+ margin-bottom: 1px; >+ border-color: ThreeDFace; >+ background-color: -moz-Field; >+ font: -moz-list; >+ -moz-appearance: menulist; >+ border-style: inset; >+ padding-right: 0px; >+ border-width: 2px; >+ padding-bottom: 0px; >+ padding-top: 1px; >+} This is quite similar to html|span.-moz-select1-container. Could you merge the CSS rules? (Just use different selectors) >+ >+html|input.-moz-xforms-date-dropdown { >+ width: 12px; >+ height: 1.3em; >+ background-image: url("data:image/gif;base64,R0lGODlhBwAEAIAAAAAAAP%2F%2F%2FyH5BAEAAAEALAAAAAAHAAQAAAIIhA%2BBGWoNWSgAOw%3D%3D") !important; >+ background-repeat: no-repeat !important; >+ background-position: center !important; >+ vertical-align: text-top; >+ margin: 0px !important; >+ margin-top: -1px !important; >+ -moz-appearance: menulist-button; >+ -moz-user-select: none !important; >+ -moz-user-focus: ignore !important; >+} And this is almost like html|input.-moz-xforms-select1-dropdown

Comment on attachment 201446 [details] [diff] [review] new patch >+ <html:span class="-moz-date-container"> >+ <html:input anonid="control" class="-moz-xforms-date-input" >+ onblur="this.parentNode.parentNode.delegate.value = this.value;" >+ onclick="this.parentNode.parentNode._change();" _change() method is no-op in this binding, so do you need this for anything? >+ >+ <method name="_change"> >+ <body> >+ </body> >+ </method> Actually, why do you need this method here at all? >+ >+ var value = this.inputField.value; >+ // js date likes YYYY/MM/DD, schema's is YYYY-MM-DD >+ value = value.replace(/-/g, "/"); Could we support both styles, js and schema, so that if user prefers to write the date, both styles can be used... But using only schema is ok too. >+ >+ <method name="_buildUI"> >+ <parameter name="aDate"/> >+ <body> >+ <![CDATA[ >+ var xhtmlNS = "http://www.w3.org/1999/xhtml"; >+ >+ // shortname defaults >+ var dayShort = ["Sun", "Mon", "Tue", "Wed", "Thu", "Fri", "Sat"]; >+ >+ // try to get localized short names. >+ // May 2005's first day is a Sunday - also, month is 0-indexed in JS >+ var day; >+ for (var i = 0; i < 7; i++) { >+ day = new Date(2005, 4, i+1).toLocaleFormat("%a"); >+ if (day) >+ dayShort[i] = day; >+ } It would be great to able to use the locale 'first-day-of-the-week' (which for me should be Monday), but I think Javascript doesn't provide such feature. So for now this is ok too. The widget looks good to me and seems to even work and is localized, so with those comments, r=me One thing we might want to add (later) is an icon for calendar. So not to use dropdown button but some icon which looks a bit like a calendar.

aaronlev, could you try this patch out from a accessibility/usability perspective, please? I can provide you with a .xpi with this patch in it if you don't build xforms anymore. Thanks.

Usability/behavior/visual issues: 1) If the calendar dropdown is down, should the user be able to enter in the input field part of the combo box? 2) If you enter 2005-12-45, it doesn't default back to today's date, but rather goes to 2006-01-14. 3) If change value of control bound to gMonth and then hit tab, the select1 input field loses the selected value (even though it is still in the instance data). 4) nit: bottom border of the input field part of the select1 for a xsd:date input is lost. Maybe ask smaug about how to fix this...seems to me this happened with select1's while he was developing them. 5) if you click to get the popdown for the xsd:date input and then click to get the popdown for the xsd:gDay, the calendar doesn't hide itself. 6) I personally think that if you click on the left or right month buttons on the calendar, then focus should stay there until a day is clicked on or the user hits the up/down key. Hitting enter key should when focus is on a left/right month button on the calendar should advance/go back a month yet keep the focus on the same button. Also if the focus is on the left month button and the user hits the right key, then focus should go to the right month button, not a day on the calendar. 7) Please update the testcase to use pseudoclass instead of attribute. 8) you should probably put alternate text on the left/right month buttons anonymous content for screen readers, shouldn't you? We can fix/resolve these issues on a seperate bug if you want. Just let me know and I'll give this code a review if you want to do that.

Comment on attachment 201446 [details] [diff] [review] new patch removing review request. Doron is fixing some of my UI critiques in this bug.

The only thing I couldn't get to work is if you focus away from the picker, it won't hide the dropdown, since it is hard to get the current focused element during a blur event.

Comment on attachment 202146 [details] [diff] [review] fix aaron's comments removing the review request. Doron's going to try xforms-previous, xforms-next to see if he can use them to resolve the dropdown issue.

Comment on attachment 202420 [details] [diff] [review] new patch not updating the instance data, so removing review request while waiting for new patch.

Comment on attachment 202420 [details] [diff] [review] new patch accessor stuff broke this.

Comment on attachment 203023 [details] [diff] [review] fix windows alignment issue for the calendar image. >+input[mozType|type="http://www.w3.org/2001/XMLSchema#date"] html|input[anonid="dropmarker"] { >+ min-width:27px; >+ min-height: 1.3em; >+ background-image: url(chrome://xforms/content/calendar.png) !important; >+ background-position: center !important; >+ background-repeat: no-repeat !important; >+} Perhaps some alt text or @title for the dropmarker button since this is a .png? Something for the accessibility agents to chew on. >+ <method name="_showPicker"> >+ <body> >+ <![CDATA[ >+ // show the picker >+ var picker = this.picker; >+ >+ if (this._isPickerVisible) { >+ this._hidePicker(true); >+ return; >+ } If we have a showPicker and a hidePicker, shouldn't we just return if showPicker is called when the picker is already showing? showPicker probably shouldn't be a toggle if there is a hidePicker. >+ >+ picker.style.display = "block"; >+ this._isPickerVisible = true; >+ >+ var value = this.inputField.value; >+ // js date likes YYYY/MM/DD, schema's is YYYY-MM-DD >+ value = value.replace(/-/g, "/"); >+ >+ // we check if the delgate is valid since javascript Date() >+ // returns a valid date for 2005-04-56. >+ var tmpDate = new Date(value); >+ if (!this.accessors.isValid() || tmpDate == "Invalid Date") >+ this._date = new Date(); >+ else >+ this._date = tmpDate; >+ >+ if (!this._uibuilt) >+ this._buildUI(this._date); >+ >+ // position the dropdown, aligning it witht the calendar button nit: spelling >+ var dropmarker = document.getAnonymousElementByAttribute(this, "anonid", "dropmarker"); >+ var dropmarkerBox = document.getBoxObjectFor(dropmarker); >+ var width = document.getBoxObjectFor(picker).width; >+ >+ var position = dropmarkerBox.x - width + dropmarkerBox.width; >+ >+ // reset position if it will bleed to the left or right >+ if (position < 0) { >+ position = 0; >+ } else if ((position + width) > window.document.width) { >+ position = window.document.width - width; >+ } in the 'else if' test, what if the result is position < 0? Is it more important that we see the left of the calendar, or the right? Test this out on your own. If you size the width of the browser with your testcase smaller, you'll see that picker creeps left away from the dropmarker button way before the dropmarker is out of the browser window. Maybe allow the drop down to move left, but only as far left as the right edge of the drop down matching the right edge of the input field? Gotta run. I'll finish the review a little later this afternoon.

Comment on attachment 203023 [details] [diff] [review] fix windows alignment issue for the calendar image. >+++ extensions/xforms/resources/content/xforms.xml 14 Nov 2005 19:44:35 -0000 >@@ -88,11 +88,11 @@ > > <property name="accessors" readonly="true"> > <getter> >- <![CDATA[ >+ <![CDATA[ > if (!this._accessors && this.delegate) > this._accessors = this.delegate.getXFormsAccessors(); > return this._accessors; >- ]]> >+ ]]> > </getter> > </property> nit: indentation. We need to decide on our indentation w.r.t. this. xforms.xml does it differently than select1.xml which does it differently from select.xml. Which is right as far as Mozilla styling? Should we always indent <![CDATA[ and should we indent the contents of the CDATA? This also applies to the rest of the code below. >+ <method name="selectCell"> >+ <parameter name="aCellNum"/> >+ <body> >+ <![CDATA[ >+ this._cells[this._currentCellIndex].node.setAttribute("tabindex", "-1"); >+ >+ this._currentCellIndex = aCellNum; >+ >+ this._cells[this._currentCellIndex].node.setAttribute("tabindex", "0"); >+ this._cells[this._currentCellIndex].node.focus(); >+ ]]> >+ </body> >+ </method> Shouldn't you check to see if aCellNum == this._currentCellIndex before you do all of this work? >+ <handlers> >+ <handler event="keypress"> >+ <![CDATA[ >+ if (this._isPickerVisible) { >+ var index = this._currentCellIndex; >+ var currentElement = event.originalTarget; >+ >+ if (event.keyCode == event.DOM_VK_DOWN) { >+ if (currentElement.localName == "input") { >+ // if we are on the button, down should focus the current selected >+ // cell >+ this.selectCell(this._currentCellIndex); >+ } else if ((index + 7) < this._cells.length) { >+ this.selectCell(index + 7); >+ } >+ } else if (event.keyCode == event.DOM_VK_UP) { >+ // td means we are on a cell >+ if (currentElement.localName == "td" && (index - 7) >= 0) { >+ this.selectCell(index - 7); >+ } else { >+ // focus the back button >+ document.getAnonymousElementByAttribute(this, "anonid", "back-button").focus(); >+ } >+ } else if (event.keyCode == event.DOM_VK_LEFT) { >+ // ctrl-left goes back a month >+ if (event.ctrlKey) { >+ this.goBack(); >+ } else if (currentElement.localName == "input") { >+ // input means we are on one of the back/fwd buttons >+ document.getAnonymousElementByAttribute(this, "anonid", "back-button").focus(); >+ } else if ((index - 1) >= 0) { >+ this.selectCell(index - 1); >+ } >+ } else if (event.keyCode == event.DOM_VK_RIGHT) { >+ // ctrl-right goes forward a month >+ if (event.ctrlKey) { >+ this.goForward(); >+ } else if (currentElement.localName == "input") { >+ // input means we are on one of the back/fwd buttons >+ document.getAnonymousElementByAttribute(this, "anonid", "fwd-button").focus(); >+ } else if ((index + 1) < this._cells.length) { >+ this.selectCell(index + 1); >+ } >+ } else if (event.keyCode == event.DOM_VK_RETURN && >+ event.originalTarget.localName == "td") { >+ var type = event.originalTarget.className; >+ if (type == "currentMonth") { >+ this.selectCell(event.originalTarget.getAttribute("num")); >+ this._valueSet(); >+ } else if (type == "prevMonth") { >+ this.goBack(); >+ } else if (type == "nextMonth") { >+ this.goForward(); >+ } >+ } else if (event.keyCode == event.DOM_VK_ESCAPE) { >+ this._hidePicker(true); >+ } >+ } else { >+ if (event.keyCode == event.DOM_VK_DOWN) { >+ // show picker when the user presses down >+ >+ // first set the accessor value, since the input's value hasn't >+ // been validated yet, and forcing this will. >+ this.accessors.setValue(this.inputField.value); >+ >+ this._showPicker() >+ } >+ } >+ ]]> >+ </handler> Please add code to test for F4, alt+down and alt+up so that this control behaves like a select1. Or open up a seperate bug (with 'a11y:' at the start of the subject) to address this problem. >+ <!-- INPUT: Month --> >+ <binding id="xformswidget-input-month" >+ extends="chrome://xforms/content/xforms.xml#xformswidget-base"> >+ <content> >+ <children/> >+ <html:select anonid="control" xbl:inherits="style" >+ onchange="this.parentNode._change();" >+ onblur="this.parentNode._setValue()"> >+ <html:option value=""></html:option> >+ </html:select> >+ </content> Should this inherit @accesskey? >+ <!-- INPUT: Day --> >+ <binding id="xformswidget-input-day" >+ extends="chrome://xforms/content/xforms.xml#xformswidget-base"> >+ <content> >+ <children/> >+ <html:select anonid="control" xbl:inherits="style" >+ onchange="this.parentNode._change();" >+ onblur="this.parentNode._setValue()"> >+ <html:option value=""></html:option> >+ </html:select> >+ </content> Should this inherit @accesskey? r-'ing to see what you are going to do with the position comments I made earlier. Other than the other minor stuff, looks great!

Comment on attachment 203418 [details] [diff] [review] forgot a file alt+up, alt+down and F4 should toggle the dropdown. Not just show it. Rest of this looks great. With that fixed, r=me.

asking for re-review since a lot has changed.

Comment on attachment 203821 [details] [diff] [review] with final nits >+ <html:tbody anonid="tbody"> >+ <html:tr> >+ <html:td colspan="1"> >+ <html:input type="button" anonid="back-button" >+ class="-moz-date-back-button" title="Previous Month" >+ onclick="this.parentNode.parentNode.parentNode.parentNode.parentNode.parentNode.goBack(true);"/> >+ </html:td> >+ <html:td colspan="5" align="center"> >+ <html:span anonid="date"/> >+ </html:td> >+ <html:td colspan="1"> >+ <html:input type="button" anonid="fwd-button" >+ class="-moz-date-fwd-button" title="Next Month" If we really need the title attribute for previous and next month, the value of it should be localizable (sp?). So either remove title or make it localizable. With either one r=me

checked into trunk

checked into MOZILLA_1_8_BRANCH via bug 323691. Leaving open for now until it gets into 1.8.0

verfied fixed on MOZILLA_1_8_BRANCH