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

fix: add support for getting initial value from slotted fields #12

Merged
merged 1 commit into from
Oct 15, 2019

Conversation

Haprog
Copy link
Contributor

@Haprog Haprog commented Oct 15, 2019

Additionally:

  • Set custom field value format methods earlier (via data binding) so that the first value-changed event from custom field already uses the correct format (with 'T' instead of '\t')
  • When setting custom field value from date time picker, use this.value instead of formattedValue for more consistent formatting (to avoid unnecessarily adding :00.000 or .000 to end of time in more cases until we have support for step property which should handle this better)

Fixes #11.

Additionally:
- Set custom field value format methods earlier (via data binding) so that the first `value-changed` event from custom field already uses the correct format (with 'T' instead of '\t')
- When setting custom field value from date time picker, use `this.value` instead of `formattedValue` for more consistent formatting (to avoid unnecessarily adding `:00.000` or `.000` to end of time in more cases until we have support for `step` property which should handle this better)

Fixes #11.
@CLAassistant
Copy link

CLAassistant commented Oct 15, 2019

CLA assistant check
All committers have signed the CLA.

@vaadin vaadin deleted a comment from CLAassistant Oct 15, 2019
Comment on lines -261 to +287
this.$.customField.value = formattedValue;
// Setting the customField.value below triggers validation of the date picker and time picker.
// If the inputs are slotted (e.g. when using the Java API) and have an initial value this can
// happen before date picker ready() which triggers an error when date picker is trying to read
// `this.$.input` (as a result of value change triggered by setting custom field value).
// Workaround the problem by overriding customField.validate while setting the initial value.
if (!this.$.customField.inputs[0].$) {
const validate = this.$.customField.validate;
/* istanbul ignore next */
this.$.customField.validate = () => true;

this.$.customField.value = this.value;

this.$.customField.validate = validate;
setTimeout(() => {
this.$.customField.validate();
});
} else {
this.$.customField.value = this.value;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kind of ugly, but I couldn't think of any nicer way to solve the problem right now. At least this seems to work and it would be good to get merged and released as alpha2 soon because it's blocking vaadin-date-time-picker-flow MVP.

Maybe this could be avoided if custom field would handle this situation automatically and not calling the validation of the inputs too early, or maybe date-picker should handle it by not trying to read the $.input if validate is called too early. I'm not sure should it be allowed to trigger validation before ready() or not.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just create a ticket to find a proper solution and refactoring this so we don't forget it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will create an issue right after merging this so I can link to the code in master branch.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Issue here #13

@Haprog Haprog requested a review from pekam October 15, 2019 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add support for getting initial value from slotted fields
3 participants