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

Server-side date parsing #2466

Open
rolfsmeds opened this issue Dec 28, 2021 · 5 comments · May be fixed by #6743
Open

Server-side date parsing #2466

rolfsmeds opened this issue Dec 28, 2021 · 5 comments · May be fixed by #6743
Assignees
Labels
enhancement New feature or request vaadin-date-picker

Comments

@rolfsmeds
Copy link
Contributor

Describe your motivation

Some applications use various specialized strings for quickly entering dates, such as +1 or tomorrow for tomorrow. While we can extend the parsing formats supported by Date Picker via date-fns in the future, it is likely that there will always be custom, application-specific strings that even date-fns does not support. Providing a solution for server-side parsing of dates, that allow developers to provide their own parsing, would solve these use cases.

Describe the solution you'd like

A way to provide server-side date parsing as a fallback (if the parsing formats provided are unable to parse the entered string). The API could be something along the lines of

DatePicker dp = new DatePicker();
dp.setFallbackParser(s -> {
  // custom logic for parsing the input string, e.g.
  if (s.equals("tomorrow")) {
    return LocalDate.now().plusDays(1);
  } else {
    return null;
}
@knoobie
Copy link
Contributor

knoobie commented Dec 28, 2021

Flow has Converter (https://github.com/vaadin/flow/blob/master/flow-data/src/main/java/com/vaadin/flow/data/converter/Converter.java) for this kind of things - the generic lambda could probably be some kind of "one way" (or two way) Converter to allow the Result to contain an error for failed fallback parsing.

public void setFallbackParser(Converter<String, LocalDate> fallbackParser);

Edit: V8 had a similar handling with Result<LocalDate> as return parameter for handleUnparsableDateString ( https://github.com/vaadin/framework/blob/7dbb4f029f2802797f531abff4dc3cf5e336e53e/server/src/main/java/com/vaadin/ui/AbstractDateField.java#L896)

roastedcpu pushed a commit to roastedcpu/flow-components that referenced this issue Jan 11, 2022
Cherry-pick of: vaadin#2409
Fixes vaadin#2466
Fixes vaadin#2319

Co-authored-by: Sascha Ißbrücker <[email protected]>
Co-authored-by: Sascha Ißbrücker <[email protected]>

(cherry picked from commit 56c878a)

Co-authored-by: Tatu Lund <[email protected]>
@vursen
Copy link
Contributor

vursen commented Nov 6, 2023

Here I described how this feature could potentially be implemented after the validation refactor is done.

@vursen vursen self-assigned this Oct 23, 2024
@vursen
Copy link
Contributor

vursen commented Oct 24, 2024

We are considering going with Rolf's original API design where the parser returns either a LocalDate or null. A null return value will indicate unparsable input and trigger the i18n error message for bad input.

datePicker.setI18n(new DatePickerI18n()
  .setBadInputErrorMessage("Invalid date format")); // Will be displayed if the parser returns `null`

datePicker.setFallbackParser(s -> {
  if (s.equals("tomorrow")) {
    return LocalDate.now().plusDays(1);
  } else {
    return null;
}

The reasoning behind not adding error message support to the parser now is that there are already two existing approaches: i18n and setErrorMessage(). Adding a third approach seems likely to make the error handling mechanism harder to understand. That said, even without direct support, it will still be possible to display different error messages based on the parser logic using this workaround:

datePicker.setFallbackParser(s -> {
  if (s.equals("tomorrow")) {
    return LocalDate.now().plusDays(1);
  } else if (s.equals("tomo")) {
    datePicker.getI18n().setBadInputErrorMessage("Must be tomorrow");
    return null;
  } else {
    datePicker.getI18n().setBadInputErrorMessage("Invalid date format");
    return null;
  }
}

If you have a different view, please feel free to share your perspective and a use case.

@knoobie
Copy link
Contributor

knoobie commented Oct 24, 2024

While I agree that "yet" another error handling mechanism is harder to understand - the mentioned "another" error handling mechanismn is the standard mechanism to be used in every flow project when you use validator or converter with Binder where the result (Result.ok(date) or Result.error("String")) is used. Additionally your workaround looks like it could create interesting side effects because it relies on the fact that getI18n() is properly updated (in time!) before the return value is evaluated.. and I doubt that anyone is going to find that "workaround" ;)

@simasch I think we talked about this feature in the past.. you might be interested in this as well

Edit: If it's a technical limitation.. or it would make it easier for you to handle it.. I would propose the following:

add fallbackErrorMessage to the i18n object.. which you can automatically set from the returning Result.error(Str)and do you refresh i18n magic if necessary. But the flow API should stay consistent for Users in my opinion that are used to Converter/Validator with Result<?> return values.

@simasch
Copy link

simasch commented Oct 24, 2024

I share @knoobie opinion. The fallback parser should not set the error message directly but return the error message or a date.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request vaadin-date-picker
Projects
Status: Under consideration
Development

Successfully merging a pull request may close this issue.

4 participants