Last Comment Bug 283344 - XForms Input control, when bound to a date value, should render as a datetimepicker
: XForms Input control, when bound to a date value, should render as a datetime...
Status: RESOLVED FIXED
: fixed1.8.0.2, fixed1.8.1
Product: Core Graveyard
Classification: Graveyard
Component: XForms (show other bugs)
: Trunk
: All All
: -- enhancement (vote)
: ---
Assigned To: Doron Rosenberg (IBM)
: Stephen Pride
Mentors:
Depends on: 92174
Blocks:
  Show dependency treegraph
 
Reported: 2005-02-23 10:20 PST by Andrew Douglas
Modified: 2016-07-15 14:46 PDT (History)
5 users (show)
See Also:
QA Whiteboard:
Iteration: ---
Points: ---


Attachments
current progress (14.06 KB, patch)
2005-09-15 15:16 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
uses a dropdown arrow like select1 (15.44 KB, patch)
2005-09-16 12:58 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
now looks like a regular select, ctrl-left/right moves back/fwd a month (17.82 KB, patch)
2005-09-20 09:06 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
as andrew pointed out, my leap year code wasn't complete (17.93 KB, patch)
2005-09-21 08:22 PDT, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
patch - date, month, year (23.29 KB, patch)
2005-10-27 13:50 PDT, Doron Rosenberg (IBM)
bugs: review-
Details | Diff | Splinter Review
testcase for all 3 types (1.46 KB, application/xhtml+xml)
2005-10-28 08:29 PDT, Doron Rosenberg (IBM)
no flags Details
new patch (23.12 KB, patch)
2005-10-31 11:47 PST, Doron Rosenberg (IBM)
bugs: review+
Details | Diff | Splinter Review
fix aaron's comments (24.20 KB, patch)
2005-11-07 13:35 PST, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
new patch (26.09 KB, patch)
2005-11-09 14:02 PST, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
calendar img (belongs in extensions/xforms/resources/content/ (197 bytes, image/png)
2005-11-09 14:03 PST, Doron Rosenberg (IBM)
no flags Details
with accessor stuff added (28.95 KB, patch)
2005-11-11 10:25 PST, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
fix windows alignment issue for the calendar image. (26.38 KB, patch)
2005-11-14 11:49 PST, Doron Rosenberg (IBM)
aaronr: review-
Details | Diff | Splinter Review
fix all nits/issues (26.66 KB, patch)
2005-11-17 10:01 PST, Doron Rosenberg (IBM)
no flags Details | Diff | Splinter Review
forgot a file (27.24 KB, patch)
2005-11-17 10:05 PST, Doron Rosenberg (IBM)
aaronr: review+
Details | Diff | Splinter Review
with final nits (28.04 KB, patch)
2005-11-21 10:58 PST, Doron Rosenberg (IBM)
bugs: review+
Details | Diff | Splinter Review

Description Andrew Douglas 2005-02-23 10:20:10 PST
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
Comment 1 Doron Rosenberg (IBM) 2005-09-15 15:16:40 PDT
Created attachment 196250 [details] [diff] [review]
current progress

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
Comment 2 Doron Rosenberg (IBM) 2005-09-16 12:58:18 PDT
Created attachment 196354 [details] [diff] [review]
uses a dropdown arrow like select1

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.
Comment 3 Doron Rosenberg (IBM) 2005-09-20 09:06:25 PDT
Created attachment 196798 [details] [diff] [review]
now looks like a regular select, ctrl-left/right moves back/fwd a month

Review comments welcome, this can be tested.
Comment 4 Doron Rosenberg (IBM) 2005-09-21 08:22:32 PDT
Created attachment 196914 [details] [diff] [review]
as andrew pointed out, my leap year code wasn't complete
Comment 5 Doron Rosenberg (IBM) 2005-10-27 13:50:27 PDT
Created attachment 201044 [details] [diff] [review]
patch - date, month, year

supports date, gMonth and gDay.
Comment 6 Doron Rosenberg (IBM) 2005-10-28 08:29:18 PDT
Created attachment 201134 [details]
testcase for all 3 types
Comment 7 Olli Pettay [:smaug] 2005-10-28 10:40:10 PDT
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 8 Doron Rosenberg (IBM) 2005-10-31 11:47:21 PST
Created attachment 201446 [details] [diff] [review]
new patch
Comment 9 Olli Pettay [:smaug] 2005-10-31 13:39:18 PST
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.
Comment 10 aaronr 2005-11-03 15:56:27 PST
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.
Comment 11 aaronr 2005-11-03 16:13:12 PST
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 12 aaronr 2005-11-04 11:43:49 PST
Comment on attachment 201446 [details] [diff] [review]
new patch

removing review request.  Doron is fixing some of my UI critiques in this bug.
Comment 13 Doron Rosenberg (IBM) 2005-11-07 13:35:59 PST
Created attachment 202146 [details] [diff] [review]
fix aaron's comments

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 14 aaronr 2005-11-08 14:27:54 PST
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 15 Doron Rosenberg (IBM) 2005-11-09 14:02:53 PST
Created attachment 202420 [details] [diff] [review]
new patch
Comment 16 Doron Rosenberg (IBM) 2005-11-09 14:03:36 PST
Created attachment 202421 [details]
calendar img (belongs in extensions/xforms/resources/content/
Comment 17 aaronr 2005-11-11 10:01:38 PST
Comment on attachment 202420 [details] [diff] [review]
new patch

not updating the instance data, so removing review request while waiting for new patch.
Comment 18 Doron Rosenberg (IBM) 2005-11-11 10:22:00 PST
Comment on attachment 202420 [details] [diff] [review]
new patch

accessor stuff broke this.
Comment 19 Doron Rosenberg (IBM) 2005-11-11 10:25:36 PST
Created attachment 202692 [details] [diff] [review]
with accessor stuff added
Comment 20 Doron Rosenberg (IBM) 2005-11-14 11:49:38 PST
Created attachment 203023 [details] [diff] [review]
fix windows alignment issue for the calendar image.
Comment 21 aaronr 2005-11-16 12:12:52 PST
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 22 aaronr 2005-11-16 16:12:01 PST
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 23 Doron Rosenberg (IBM) 2005-11-17 10:01:19 PST
Created attachment 203417 [details] [diff] [review]
fix all nits/issues
Comment 24 Doron Rosenberg (IBM) 2005-11-17 10:05:41 PST
Created attachment 203418 [details] [diff] [review]
forgot a file
Comment 25 aaronr 2005-11-18 13:36:39 PST
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.
Comment 26 Doron Rosenberg (IBM) 2005-11-21 10:58:20 PST
Created attachment 203821 [details] [diff] [review]
with final nits

asking for re-review since a lot has changed.
Comment 27 Olli Pettay [:smaug] 2005-11-21 12:02:41 PST
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
Comment 28 Doron Rosenberg (IBM) 2005-11-22 10:54:00 PST
checked into trunk
Comment 29 aaronr 2006-02-02 17:25:54 PST
checked into MOZILLA_1_8_BRANCH via bug 323691.  Leaving open for now until it gets into 1.8.0
Comment 30 aaronr 2006-07-07 11:27:55 PDT
verfied fixed on MOZILLA_1_8_BRANCH

Note You need to log in before you can comment on or make changes to this bug.