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: do not invalidate when clearing a valid value #5579

Merged
merged 9 commits into from
Oct 16, 2023

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Oct 13, 2023

Description

The PR prevents field components that support bad input from invalidating when clearing a valid value programmatically.

Depends on

Fixes #5462

Part of #5537

Type of change

  • Bugfix

@vursen vursen force-pushed the fix/do-not-validate-on-clear branch from 7409383 to a828ba7 Compare October 13, 2023 12:34
@vursen vursen changed the title fix: do not invalidate when clearing a valid value fix: do not invalidate when a valid value is cleared Oct 13, 2023
@vursen vursen changed the title fix: do not invalidate when a valid value is cleared fix: do not invalidate when clearing a valid value Oct 13, 2023

// When the value is cleared programmatically, reset hasInputValue
// so that the following validation doesn't treat this as bad input.
if (Objects.equals(value, getEmptyValue())) {
Copy link
Contributor Author

@vursen vursen Oct 13, 2023

Choose a reason for hiding this comment

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

note: Even though the issue doesn't manifest in DatePicker because its web component handles hasInputValue differently from other components, I've decided that it won't hurt to include the fix in DatePicker as well in case the web component's implementation changes in the future.

@vursen vursen force-pushed the fix/do-not-validate-on-clear branch from 4375ff3 to 8454b18 Compare October 16, 2023 06:49
@vursen vursen marked this pull request as ready for review October 16, 2023 06:49
@vursen vursen requested a review from tomivirkki October 16, 2023 06:49
@vursen vursen marked this pull request as draft October 16, 2023 08:15
@vursen vursen force-pushed the fix/do-not-validate-on-clear branch 6 times, most recently from c2a4548 to d488f2b Compare October 16, 2023 11:04
@vursen vursen force-pushed the fix/do-not-validate-on-clear branch from d488f2b to 84d906a Compare October 16, 2023 11:04
@vursen vursen marked this pull request as ready for review October 16, 2023 11:05
@vursen vursen requested review from tomivirkki and removed request for tomivirkki October 16, 2023 11:05

// When the value is cleared programmatically, reset hasInputValue
// so that the following validation doesn't treat this as bad input.
if (valueEquals(value, getEmptyValue())) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: this "intensive calculation" is done twice for the same params :) (all setValue methods)

Copy link
Contributor Author

@vursen vursen Oct 16, 2023

Choose a reason for hiding this comment

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

Thanks, extracted into a var.

@vursen vursen enabled auto-merge (squash) October 16, 2023 14:07
@vursen vursen merged commit bade42a into main Oct 16, 2023
@vursen vursen deleted the fix/do-not-validate-on-clear branch October 16, 2023 14:21
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
20.1% 20.1% Duplication

idea Catch issues before they fail your Quality Gate with our IDE extension sonarlint SonarLint

@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 24.3.0.alpha1 and is also targeting the upcoming stable 24.3.0 version.

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

Successfully merging this pull request may close these issues.

IntegerField / BigDecimalField .clear sets state to invalid
4 participants