Dateinput change event happens before value is set #292

Closed
bradrobertson opened this Issue Jan 13, 2011 · 6 comments

Comments

Projects
None yet
3 participants
@bradrobertson
Contributor

bradrobertson commented Jan 13, 2011

I'm not sure if this is on purpose, but there's no afterChange event which makes me think this is a bug. The following jsFiddle illustrates the issue: http://jsfiddle.net/7NNgP/

When a user clicks the calendar, effectively 'changing' the date, the actual value of the input at this time is the value 'before' the date was selected. Is this on purpose?

I would expect that the input value would be the new value set by the calendar. The issue arrises for me as I have a change handler that submits a form, but the value submitted from this date is not the value that the user selected, so each change submits the previous date value selected, which is obviously incorrect.

Two workarounds are possible. Trigger 'change' after the value is set (though this gets awkward as it won't fire if e.isDefaultPrevented is true) or provide another event that is triggered after the value has changed. I've submitted a pull request with the latter option

@ches

This comment has been minimized.

Show comment Hide comment
@ches

ches Sep 9, 2011

Hey Brad,

This is a really annoying bug and unless I'm mistaken it looks like the fix still isn't in the dev branch. #230 is also a duplicate of this that could be closed, and there is a suggested fix on that ticket as well.

I think jQuery users are pretty accustomed to something close to this being the standard pattern for an auto-submitting input, so it'd be great if the change event worked rather than afterChange IMO:

$('#input').change(function() {
  $(this).closest('form').submit();
}

Any chance we could get this fixed for 1.2.6?

ches commented Sep 9, 2011

Hey Brad,

This is a really annoying bug and unless I'm mistaken it looks like the fix still isn't in the dev branch. #230 is also a duplicate of this that could be closed, and there is a suggested fix on that ticket as well.

I think jQuery users are pretty accustomed to something close to this being the standard pattern for an auto-submitting input, so it'd be great if the change event worked rather than afterChange IMO:

$('#input').change(function() {
  $(this).closest('form').submit();
}

Any chance we could get this fixed for 1.2.6?

@bradrobertson

This comment has been minimized.

Show comment Hide comment
@bradrobertson

bradrobertson Sep 10, 2011

Contributor

ya i'll look at it this weekend. I posted this a while ago (before I was a committer to the project) and subsequently forgot about it. I'll try and get this done for the weekend, we're going to be releasing all the latest changes asap!

Contributor

bradrobertson commented Sep 10, 2011

ya i'll look at it this weekend. I posted this a while ago (before I was a committer to the project) and subsequently forgot about it. I'll try and get this done for the weekend, we're going to be releasing all the latest changes asap!

@ches

This comment has been minimized.

Show comment Hide comment
@ches

ches Sep 10, 2011

Awesome, thanks! I figured that was the case :-)

ches commented Sep 10, 2011

Awesome, thanks! I figured that was the case :-)

@alibby251

This comment has been minimized.

Show comment Hide comment
@alibby251

alibby251 Sep 10, 2011

Contributor

Copying in the request and code fix from #230, to remove duplication:

"The documenation on events says that jQueryTools uses a beforeEvent and Event pattern for extending standard events. This, I suspected that the author intended (changes tagged with "RDR").

NOTE: This change might break some code which was expecting the html input element to still have the previous value when change event raised.

Also, W3C documentation suggests that with <input ... onChange="XXX"/> , XXX is called AFTER the input's value have been updated. Which is what this change does.

//{{{ pick
    function select(date, conf, e) {

        // current value
        value = date;
        currYear = date.getFullYear();
        currMonth = date.getMonth();
        currDay = date.getDate();


        // change
        e = e || $.Event("api");
        e.type = "beforeChange"; // RDR

        fire.trigger(e, [date]);
        if (e.isDefaultPrevented()) { return; }

        // formatting           
        input.val(format(date, conf.format, conf.lang));

        // store value into input
        input.data("date", date);

        e.type = "change"; // RDR
        fire.trigger(e);

        self.hide(e);
    }
    //}}}

"

Contributor

alibby251 commented Sep 10, 2011

Copying in the request and code fix from #230, to remove duplication:

"The documenation on events says that jQueryTools uses a beforeEvent and Event pattern for extending standard events. This, I suspected that the author intended (changes tagged with "RDR").

NOTE: This change might break some code which was expecting the html input element to still have the previous value when change event raised.

Also, W3C documentation suggests that with <input ... onChange="XXX"/> , XXX is called AFTER the input's value have been updated. Which is what this change does.

//{{{ pick
    function select(date, conf, e) {

        // current value
        value = date;
        currYear = date.getFullYear();
        currMonth = date.getMonth();
        currDay = date.getDate();


        // change
        e = e || $.Event("api");
        e.type = "beforeChange"; // RDR

        fire.trigger(e, [date]);
        if (e.isDefaultPrevented()) { return; }

        // formatting           
        input.val(format(date, conf.format, conf.lang));

        // store value into input
        input.data("date", date);

        e.type = "change"; // RDR
        fire.trigger(e);

        self.hide(e);
    }
    //}}}

"

@bradrobertson

This comment has been minimized.

Show comment Hide comment
@bradrobertson

bradrobertson Sep 10, 2011

Contributor
Contributor

bradrobertson commented Sep 10, 2011

@ches

This comment has been minimized.

Show comment Hide comment
@ches

ches Sep 10, 2011

Woohoo!

ches commented Sep 10, 2011

Woohoo!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment