Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Entering date with keyboard does not work ($50 bounty to fix) #231

Closed
sjordan1975 opened this issue Jun 14, 2017 · 20 comments
Closed

Entering date with keyboard does not work ($50 bounty to fix) #231

sjordan1975 opened this issue Jun 14, 2017 · 20 comments

Comments

@sjordan1975
Copy link

sjordan1975 commented Jun 14, 2017

I have a fiddle here: https://jsfiddle.net/sjordan/r81k8240/5/

When I try to enter a date using just the keyboard (tab and number pad) it does not work.

Repro steps:

  1. Load fiddle at URL in Chrome
  2. Place mouse over "mm" in input field and type 10 (the month 10 is entered and auto tabs to "dd")
  3. Type 31 (the day 31 is entered and auto tabs to "yyyy")
  4. Attempt to type 2017

I expect to get 10/31/2017; however, in my case I get 10/31/1907

However, doing the above steps with default HTML5 works; for example, perform the above steps with W3Schools test form: https://www.w3schools.com/html/tryit.asp?filename=tryhtml_input_date

NOTE: I have placed a $50 bounty to get this fixed urgently: https://www.bountysource.com/issues/46214782-entering-date-with-keyboard-does-not-work

@sjordan1975 sjordan1975 changed the title Entering date with keyboard does not work Entering date with keyboard does not work ($50 bounty to fix) Jun 15, 2017
@kingjan1999
Copy link

kingjan1999 commented Jun 15, 2017

Hey,

i don't have a solution (yet), but I can offer you a possible workaround:

Use a different model for the field and the set() method. Unfortunately, this prevents the validation of the field, so you have to set required to false and validate the field in the onSubmit method.

I know this is far from perfect but it might help you.

Edit: see fiddle

@kingjan1999
Copy link

Oh and you might consider using the latest version, this seems related to #138

@sjordan1975
Copy link
Author

sjordan1975 commented Jun 15, 2017

I have tried the latest version, but doesn't seem to fix the issue; see https://jsfiddle.net/sjordan/8bph57yh/

The code you proposed seems like a decent temporary solution, but as you wrote, it's just a workaround. I do thank you @kingjan1999

@lionel-bijaoui
Copy link
Member

lionel-bijaoui commented Jun 15, 2017

You seem to be missing the bigger problem, this is not yet compatible with all browser.
We added this field to "future-proof" VFG. We clearly didn't future proof it enough. I hope you bounty will raise a nice solution.

Here are some clues to correct the problem:

/* src/fields/core line:6 */
:value="value",
@input="value = $event.target.value",

We directly use the input event to set the value, which is use as a value for the field.
While this work fine for most field, maybe we need to move all of this into it's own method and do differently for date, etc. ?

/* src/fields/core line:52 */
formatValueToField(value) {
	if (value != null) {
		let dt;
		switch(this.schema.inputType){
		case "date":
			dt = this.schema.format ? fecha.parse(value, this.schema.format) : new Date(value);
			return fecha.format(dt, "YYYY-MM-DD");
		case "datetime":
			dt = this.schema.format ? fecha.parse(value, this.schema.format) : new Date(value);
			return fecha.format(dt, "YYYY-MM-DD HH:mm:ss");
		case "datetime-local":
			dt = this.schema.format ? fecha.parse(value, this.schema.format) : new Date(value);
			return fecha.format(dt, "YYYY-MM-DDTHH:mm:ss");
		}
	}
	
	return value;
}

If a format is defined in the schema, the code use fetcha to transform this String into a Date object, else it use it as an input to a Date object.
After that, the code use that Date object and format it into the good format.
All of this before sending the value to the model. I think we did that to bridge the gap between compatibility problem.
If the field is compatible, the browser pre-format it for us but if not, we need a way to assure we are not sending garbage to the model. It unify the value before sending it to the model.
But it might be where the problem start. I would start to test what Chrome is sending here.
Then see how fetcha and Date is transforming that.
This fields also have to work with incompatible browser, please keep that in mind !

formatValueToModel(value) {
	if (value != null) {
		let m;
		switch (this.schema.inputType){
		case "date":
			m = fecha.parse(value, "YYYY-MM-DD");
			if (m !== false) {
				if (this.schema.format)
					value = fecha.format(m, this.schema.format);
				else
					value = m.valueOf();
			}
			break;
		case "datetime":
			m = fecha.parse(value, "YYYY-MM-DD HH:mm:ss");
			if (m !== false) {
				if (this.schema.format)
					value = fecha.format(m, this.schema.format);
				else
					value = m.valueOf();
			}
			break;
		case "datetime-local":
			m = fecha.parse(value, "YYYY-MM-DDTHH:mm:ss");
			if (m !== false) {
				if (this.schema.format)
					value = fecha.format(m, this.schema.format);
				else
					value = m.valueOf();
			}
			break;
		case "number":
			return Number(value);
		}
	}

	return value;
}

Here, it is the other way, when the model is defined, we change the value of the field. While, it may not be directly causing problem in your example, if the code change for value->model, it need to change for model->value too.
I hope this might help some bounty hunter trying to resolve this issue, like @kingjan1999


In the meantime, you are better of with dateTime, pikaday or a custom field (I did one using flatpickr in a few lines).

@icebob can you take a look at this when you have the time ? Thanks

@kingjan1999
Copy link

kingjan1999 commented Jun 15, 2017

I think I've found the issue (at least for this problem): fecha.
While entering the date chrome sends it to us formatted like this 0002-10-10.
We are parsing this via fecha in formatValueToField and it should output 0002-10-10 as well. But fecha parses and formats it as 1902-10-10. Sending that back to chrome results in the error described above.

Replacing fecha with moment.js (which handles the dates correctly) fixes the problem.

@lionel-bijaoui
Copy link
Member

lionel-bijaoui commented Jun 15, 2017

@kingjan1999 Great work, but we are not using moment. It is way to big and not modular enough.
Can you find a parameter for fetcha that can fix this issue instead ?
EDIT: Also, have you tired it with Firefox (not compatible with date) ?

@kingjan1999
Copy link

I'm afraid there is no such option: fecha uses getFullYear() for YYYY, but getFullYear returns just 2 instead of 0002. Either we write a workaround by formatting the dates by ourselves or report the issue to fecha and wait for a fix.

Oh, and no, it doesn't work in firefox (yet). I'm working on it.

@lionel-bijaoui
Copy link
Member

Maybe it's a good time to push this idea ?

Can we use date-fns, instead of fetcha ?

With this, would the problem be resolved ?

I'm using date-fns in my project, I find it better than fectha.
For me, date-fns is like moment.js but modern, functional and modular.
You just import the function you need, when you need it and that's it.
No bloat, no weird conversion to another format.
I'm biased of course, but when it was time for my project to find a better replacement to moment.js, I tested both library and date-fns wins hands down.

Since it is very modular, we can start to use it alongside fetcha and remove fetcha progressively.
@icebob What do you think about this ?

@kingjan1999 Thank you for your work, when you think you have something ready, don't forget to do a PR.

@kingjan1999
Copy link

kingjan1999 commented Jun 15, 2017

dateFns.format(new Date(0002,1,11), 'YYYY-MM-DD')

returns
1902-02-11

and

dateFns.getYear(new Date(0002,1,11))

in
1902

But moment isn't perfect as well, if it should work in firefox, we have to limit the valid formats (if no dateformat is given already), otherwise moment will parse almost every number as a valid date by falling back to new Date() (unless you know a method how to detect if the user have finished his input).

@sjordan1975
Copy link
Author

sjordan1975 commented Jun 16, 2017

@kingjan1999 Great investigation so far!

@kingjan1999, @lionel-bijaoui It's kind of like the return of the Y2K bug?! fetcha and date-fns interpret a year such as "02" as 1902 rather than 2002, for example?

@lionel-bijaoui
Copy link
Member

Well, the problem is that these fields are not yet compatible in all major browser and we try to fix the discrepancy with these lib.
Chrome is fine without any modification. But we still need to format the value for other browsers.

One fix that I see, would be to check if the year start with two 0 and is a 4 digit number. If that the case, we don't format or send the data to the model.
But it is going to be messy...

The best way to deal with that would be to just send the data like it is. No format/getYear.
It also mean that we need to prevent people (in the doc) that these kind of field are dependent on browser implementation.
We would just align ourselves on the reality of these kind of fields and not try to do magic bridging.

Type input was meant to be a way to use HTML5 input type after all. That way you could, at the very least, do the same thing you can with a static form.
If you want cross browser and other feature, there is plenty of choices with 2 other lib available and it is very easy to make your own custom field with a lib of your choice.

Again, this choice depends on @icebob decision.

@icebob
Copy link
Member

icebob commented Jun 16, 2017

Hi all,

As @lionel-bijaoui said the main problem is that the html5 date fields are very browser dependent fields. We can't make a solution which will be good everyone. So in this case please copy the fieldInput.js and create a custom date field with the desired features. With formatValueToModel and formatValueToField methods can be convert/format the values vice-versa.

@lionel-bijaoui
Copy link
Member

lionel-bijaoui commented Jun 16, 2017

@icebob Since input field is supposed to be very close from native/manual input, isn't better to remove any conversion from these.
And, as you said

We can't make a solution which will be good everyone

If someone want something different, they can just clone it and do their thing (conversions, etc.).

That way, when all browser will be compatible with date, datetime and datetime-local we have nothing to change, since we will not be interfering with the values and it remove all the headaches.

It is very easy to modify, we just delete lines 53->67 and 75->101 from src/fields/core/fieldInput.vue

Right now, this is kind of backward, since newer more capable browser have a penalty (a bug).

@sjordan1975
Copy link
Author

sjordan1975 commented Oct 9, 2017

@icebob Hello, my bounty offer has been open and hanging out there since June. If this issue is not going to be fixed, please close it and tag "won't fix", so that my bounty offer can be closed.

Cheers

@icebob
Copy link
Member

icebob commented Oct 10, 2017

Are there anybody?

@zoul0813
Copy link
Member

zoul0813 commented Dec 13, 2017

@sjordan1975 @icebob this should be solved with PR #360

@icebob
Copy link
Member

icebob commented Dec 13, 2017

@sjordan1975 Can you check it?

@sjordan1975
Copy link
Author

Something still does not seem right.

New fiddle

Repro steps (as original):

Load fiddle at URL in Chrome

  1. Place mouse over "mm" in input field and type 10 (the month 10 is entered and auto tabs to "dd")
  2. Type 31 (the day 31 is entered and auto tabs to "yyyy")
  3. Attempt to type 2017

I expect to get 10/31/2017; however, in my case I get 10/31/0017

@sjordan1975
Copy link
Author

@icebob Please mark this comment as "won't fix" so that my bounty can be closed. It's been over 6 months since I raised this issue and 2 weeks since I reported that the latest update seems to still have the issue. Thank you in advance

@zoul0813
Copy link
Member

zoul0813 commented Jan 2, 2018

I think we need to add more logic into the abstract field and date component. Something that detects html5 Date fields in chrome and other browsers that convert it from a standard text entry ... if detected, it never sets the field value while it has focus ... the main issue is the formatModeToField code that overwrites what’s your entering. The delay I added helps, if you type fast... but doesn’t solve the problem if you’re entering the values in slowly.

Adding some “do not update field while has focus” logic will be useful for some other fields as well.

@icebob icebob added the wontfix label Jan 2, 2018
@icebob icebob closed this as completed Jan 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants