BorisMoore / jsviews

Interactive data-driven views, MVVM and MVP, built on top of JsRender templates
http://www.jsviews.com/#jsviews
MIT License
856 stars 130 forks source link

updating property of type date with text #360

Closed ainglese closed 7 years ago

ainglese commented 7 years ago

when i try to update a property of type Date with a value of type string, nothing appens. this is due to this if: if (!(property instanceof Date) || property > value || property < value) {

https://github.com/BorisMoore/jsviews/blob/bf667cb65ddd45e9cde711b78a9022b493464848/jsviews.js#L3117

if property is of type date, dirst check is false, then if value is string, property > value is false and property < value is also false, so the "set" operation is not performed at all. i understand that setting a date with a string (that by the way represents a date) is not correct, but i think something should be notified at least..

here a test-case: https://jsfiddle.net/zv3ebjqz/3/

BorisMoore commented 7 years ago

Hmm - yes I think you are right, this needs to be changed. In fact right now, once a property has been set observably to be a date, it can no longer be observably updated. All the examples using Date, at jsviews.com are in fact using the jQuery UI datepicker (http://www.jsviews.com/#samples/tag-controls/datepicker), which actually sets the input to type string, so I didn't realise this scenario of an observable Date property was not being supported.

I think we need to change that line of code to

if (value instanceof Date || property instanceof Date || property > value || property < value) {

so that you can freely observably update a property, setting it to a Date type or a string type or some other type, independently of its current type.

Can you validate that this change works for you?

ainglese commented 7 years ago

well, actually i've made a workaround by forcing the value to be a date, witch seems to be working. i've changed the datepicker here:

onAfterLink: function (tagCtx, linkCtx) {

    .....

    $('input', tag.linkedElem).datepicker({
        onSelect: function (dateText, inst) {
            tag.value = dateText;
            var dateObj = $.datepicker.parseDate(tag._datefrmt, dateText);
            tag.update(dateObj);
        }
    });
    tag._datefrmt = $('input', tag.linkedElem).datepicker("option", "dateFormat");
    var formatted = $.datepicker.formatDate(tag._datefrmt, new Date(tagCtx.args[0]));
    tag.setValue(formatted);

}

please note that using formatDate and parseDate allow for localization of the conversion from string to date and viceversa. i think this is required since if the value we are binding to is a date, picking the date should give as a date too.

i'll give a try to your fix anyway, since a string value is better than no value.. :)

BorisMoore commented 7 years ago

Sorry, my fix was incorrect. I replaced it above by a corrected version.

BorisMoore commented 7 years ago

Fixed with commit 84

trueqbit commented 7 years ago

Hi guys,

could I reopen this discussion? The problem with Boris' fix is that now datepicker.setValue() is agnostic to whether the value is of type string or date object, but datepicker.onSelect() is not.

I am in the situation where the the data value is a date text in local ISO 8601 format, which needs to be converted and back again (additionally with some special conversion rules for exclusive ranges), plus I'd like to have a generic datepicker tag control, which is agnostic to what types are actually involved. Hence I was trying to delegate the type handling to converters (so again, this works great when setting the date value but not after selecting it).

So I propose to check whether a back converter is present and presume it can convert from a date object:

options: function() {
    var tag = this;
    return {
        onSelect: function(dateText, dp) {
            // if convertBack is present then we presume it can convert
            // a date object to the internal representation.
            // otherwise we update the bound variable with the date text
            var updateValue = tag.tagCtx.props.convertBack ?
                $.datepicker._getDate(dp) :
                dateText;
            tag.update(updateValue);
        }
    };
},

This gist shows the whole tag control.

BorisMoore commented 7 years ago

@trueqbit:

Yes, thanks... I have been doing work on both datepicker and spinner which I think will address the issues you have pointed out, here and on #374. I'll get the code to you soon...

BorisMoore commented 7 years ago

This test.zip includes updated jsviews.js, jsviews-jqueryui-widgets.js and validate.js, and an example .html page for {{datepicker}} and for {{slider}}.

With the update {{datepicker}} will by default data-bind to string data, using the current dateFormat used by the datepicker ($(element).datepicker("option", "dateFormat")).

But you can choose different formats for data and for display by setting them as properties (using jQuery UI datepicker formats):

{^{datepicker dateString dataFormat="mm-dd-yy" dateFormat="DD MM d, yy"/}}

Alternatively:

You can create your own flavor of datepicker which defaults to whatever you want, For example a datepicker which uses Date, and has default display format "DD MM d, yy":

$.views.tags("my_dp_usingDateObject", {
  baseTag: "datepicker",
  dataFormat: false, // Any falsy value
  dateFormat: "DD MM d, yy"
})

To change the default for the usual {{datepicker}} tag, use:

$.views.tags("datepicker", {
  baseTag: "datepicker",
  dataFormat: ...
  dateFormat: ...
})

You can also use your own converters for custom formats. For example, you could use moment.js to format/parse to other serialization patterns (as in the datepicker with converters sample).

If you want, you can include such converters by default in a custom tag (or override the regular {{datepicker}} tag) by writing:

$.views.tags("my_dp_usingCustomFormat", {
  baseTag: "datepicker",
  convert: function (wcfDate) {
    // Use wcfDate - formatted using moment.js, for serialization
    return $.datepicker.formatDate(this.dateFormat, moment(wcfDate).toDate());
  },
  convertBack: function (displayedDate) {
    // Use jQuery UI formats for display (http://api.jqueryui.com/datepicker/#utility-formatDate)
    return moment($.datepicker.parseDate(this.dateFormat, displayedDate)).format('/\\D\\at\\e(xZZ)/')
  },
  dateFormat: 'm-d-yy'
})

For spinners (see #374) you can create a time spinner by creating a custom tag overriding the onBind handler:

$.views.tags({
  timespinner: {
    baseTag: "spinner",
    onBind: timespinnerOnBind
  }
});

with the following onBind:

function timespinnerOnBind() {
  var tag = this;
  tag.baseApply(arguments);
  var widget = tag.widget;
  widget.options.step = 60*1000,
  widget.options.page = 60;
  widget._format = _format;
  widget._parse = _parse;
  tag.tagCtx.props.trigger = false;
}

function _format(value) {
  var d = new Date(+value);
  // take timezone offset into consideration
  d = new Date(+d + d.getTimezoneOffset()*60*1000);
  return Globalize.format(d, "t");
}

function _parse(value) {
  if (typeof value === "string") {
    if (+value == value) {
      // number string
      value = +value;
    } else {
      // date string
      var d = Globalize.parseDate(value);
      value = d ? +d - d.getTimezoneOffset()*60*1000 : 0;
    }
  }
  return value;
}

Alternatively, rather than creating a new {{timespinner}} tag, you can override onBind on the instance:

{^{spinner time onBind=~bnd/}}
BorisMoore commented 7 years ago

Commit 85, just published, includes changes and improvements to {{datepicker}} and {{spinner}}, and a new {{timespinner}}, along with documentation. See in particular:

@trueqbit - let me know if your scenarios are covered by these changes. Thanks!

trueqbit commented 6 years ago

Better late than never I want to let you know that the new datepicker control is working as expected (not to say great!) [using v0.9.90 Beta]