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

b:dateTimePicker format #654

Closed
chongma opened this issue Mar 12, 2017 · 20 comments
Closed

b:dateTimePicker format #654

chongma opened this issue Mar 12, 2017 · 20 comments
Assignees
Milestone

Comments

@chongma
Copy link
Collaborator

chongma commented Mar 12, 2017

i just tried a b:dateTimePicker with value= java.util.Date and format="dd/MM/yyyy" and i get We/03/yyyy and other strange things in the picker. with no format i get a short date. am I using the wrong date format or passing the wrong object?

@chongma
Copy link
Collaborator Author

chongma commented Mar 12, 2017

it works with "DD/MM/YYYY". is there a reason it doesn't follow the normal formatting that would usually be passed to SimpleDateFormat or f:dateTimeConverter for example?

@stephanrauh
Copy link
Collaborator

Oh. It's good that you mention this. Actually, b:dateTimePicker is this component: https://eonasdan.github.io/bootstrap-datetimepicker/Options/, which is using the formatting options of moment.js. These options are completely different from what we Java programmers are used to: http://momentjs.com/docs/#/displaying/format/

Fancy to add some lines to the documentation page?

@chongma
Copy link
Collaborator Author

chongma commented Mar 14, 2017

Moved to BootsFacesWeb project pull request

@chongma chongma closed this as completed Mar 14, 2017
@stephanrauh stephanrauh reopened this Mar 14, 2017
@stephanrauh
Copy link
Collaborator

Oops - it seems we've got a big problem. The formatting string "ddd, hA" of moment.js does not work. The method DateTimePickerRenderer::getInternalDateAsString() uses the SimpleDateformatter, passing the format strings of moment.js to it. This obviously only works with the small subset of common formatting options.

@stephanrauh stephanrauh added this to the v1.1.0 milestone Mar 14, 2017
@stephanrauh stephanrauh added this to In Progress in BootsFaces v1.1.0 Mar 14, 2017
@chongma
Copy link
Collaborator Author

chongma commented Mar 14, 2017

I closed because I thought it was only a documentation issue. It should only use moment.js or the simpledateformatter but not both? I can check that function tomorrow to see if I can be of any help with the issue?

@stephanrauh
Copy link
Collaborator

Yeah, it was a surprise to me, too. Honestly, I don't know how to solve the issue. I think the Java side simply expects a java.util.Date. Is it possible to display the date in the user-defined format, while passing it as a standardized format in the requests and responses? Such as "dd.mm.yyyy" or the number of milliseconds since 1970?

@chongma
Copy link
Collaborator Author

chongma commented Mar 15, 2017

could it use this? https://github.com/MadMG/moment-jdateformatparser
the user could define the format using Java notation and use this library to convert it to moment js format in the client at initialisation? is there already a javascript block to initialise the control?

@chongma
Copy link
Collaborator Author

chongma commented Mar 15, 2017

actually...i mean the opposite to not break backwards compatibility. the user defines using moment.js format. it gets converted internally to Java format (for the SimpleDateFormatter) then when the control is rendered the Java format gets converted back to moment.js format. Hope that makes sense! Let me know and I will create a PR

@chongma
Copy link
Collaborator Author

chongma commented Mar 15, 2017

I notice you already have functions to convert to and from moment.js. I tried this as well and it doesn't look easy to get right.
I have tested the below functions and they can format to and from a java.util.Date using a moment.js format. However they are slow because they load moment.js so it would be best if they could somehow be kept alive in a singleton or something.

public static String formatDate(Date date, String format) {
		InputStream is = Thread.currentThread().getContextClassLoader()
				.getResourceAsStream("/META-INF/resources/bsf/js/moment.min.js");
		Reader reader = new InputStreamReader(is);
		ScriptEngineManager manager = new ScriptEngineManager();
		ScriptEngine engine = manager.getEngineByName("JavaScript");
		try {
			engine.eval(reader);
			String jsFormat = format.replace("'", "\\'");
			String javascript = "var formatted = new moment(" + date.getTime() + ").format('" + jsFormat + "');";
			engine.eval(javascript);
			String formatted = (String) engine.get("formatted");
			System.out.println("FORMATTED: " + formatted + ", using: " + format);
			return formatted;
		} catch (ScriptException e) {
			throw new RuntimeException(e);
		}
	}

	public static Date formatDate(String value, String format) {
		InputStream is = Thread.currentThread().getContextClassLoader()
				.getResourceAsStream("/META-INF/resources/bsf/js/moment.min.js");
		Reader reader = new InputStreamReader(is);
		ScriptEngineManager manager = new ScriptEngineManager();
		ScriptEngine engine = manager.getEngineByName("JavaScript");
		try {
			engine.eval(reader);
			String jsFormat = format.replace("'", "\\'");
			String javascript = "var ts = moment(\"" + value + "\", \"" + jsFormat + "\").valueOf();";
			engine.eval(javascript);
			long ts = (long) engine.get("ts");
			System.out.println("FORMATTED: " + ts + ", using: " + format);
			return new Date(ts);
		} catch (ScriptException e) {
			throw new RuntimeException(e);
		}
	}

this way it could avoid ever having to convert between java and Moment.js formats. the problem now seems to be that it initialises the format to be a java format instead of a Moment.js format.

@stephanrauh
Copy link
Collaborator

This PR seems to be compatible all the way down to Java 6, so I've merged it. However, I'm a bit concerned about performance. What about caching? And could we identify the most common use cases (like American date format and continental European date format) in order to hard-wire them?

@chongma
Copy link
Collaborator Author

chongma commented Mar 16, 2017

The PR was not complete but only a demo of the concept. There need to be some changes to the way the control initialises and stores the format attribute. I would need to dig around more to fully understand that. The performance should be ok because it keeps the ScriptEngine alive in a Singleton for common use and flushes it every 2000 hits. I think it might be messy to add code checking common cases if it already works ok

@stephanrauh
Copy link
Collaborator

Yes, I was aware of the fact that the PR was more a demo than the final solution. But it already looked interesting enough to merge it, so other people can see it and comment on it.

@stephanrauh
Copy link
Collaborator

What about this one? https://github.com/MadMG/moment-jdateformatparser/blob/master/moment-jdateformatparser.js

It seems someone has already done the hard work for us!

@chongma
Copy link
Collaborator Author

chongma commented Mar 16, 2017

Yes i mentioned it in an earlier comment but it doesn't evaluate in the script engine. I also converted it into Java but it doesn't seem to give back the correct translations

@stephanrauh
Copy link
Collaborator

Seems I'm too busy :). Actually, my idea was to translate the library to Java. No idea how much work this is. But my guess is that it's not too much work. After all, the hard work is creating the hash tables.

stephanrauh added a commit that referenced this issue Mar 19, 2017
@stephanrauh
Copy link
Collaborator

stephanrauh commented Mar 19, 2017

@chongma I've just reviewed your changes. You've done a great job!

I've also started to document the format conversions. Among other things, I think an interactive cheat sheet might be useful. I'm not sure I've found the best approach yet. Would you like to have a glance at it?

Todos left:

  • describe the implicit conversions (what happens if no locale and/or no moment.js format are provided)
  • find a more attractive and more useful way to present the cheat sheet
  • add default formats

@chongma
Copy link
Collaborator Author

chongma commented Mar 20, 2017

this was my conversion of the moment-jdateformatparser.js
MdfToJdf.txt
but i saw you had already done it with LocaleUtils

@chongma
Copy link
Collaborator Author

chongma commented Mar 20, 2017

I looked at the cheat sheet. it is a really good idea. i would have preferred to keep everything in java format and do the conversion to moment js internally but as there is currently no reliable way to do this it seems logical to keep everything in moment js.

@stephanrauh
Copy link
Collaborator

stephanrauh commented Mar 20, 2017 via email

@stephanrauh
Copy link
Collaborator

@chongma @asterd @TheCoder4eu I've just committed an optimized and improved version of the localization features of the dateTimePicker. I've modified a lot of code, so I'd appreciate your help with testing. This includes the good old <b:datePicker />. Both date pickers share a lot of the i18n code.

Thanks in advance,
Stephan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

No branches or pull requests

2 participants