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!(BigDecimalField): use custom logic instead of validated event #5581

Merged
merged 8 commits into from
Oct 18, 2023

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Oct 13, 2023

Description

The PR refactors BigDecimalField to make it trigger validation whenever a value-changed event is received from the client while the value still remains empty on the server, instead of relying on the validated event.

Depends on

Warning
It's a behavior altering change because it removes validation on blur when the value hasn't changed.

Part of #5537

Type of change

  • Bugfix

@vursen vursen force-pushed the feat/big-decimal-field/remove-validated-event branch from 854687f to 77c7052 Compare October 16, 2023 12:31
@vursen
Copy link
Contributor Author

vursen commented Oct 16, 2023

I added tests for eager mode and they uncovered that there is an unexpected initial validation that is caused by the value-changed event that Polymer fires during the web component's initialization.

We've already encountered this kind of issue before: see #2878 that fixes it for DatePicker. In short, such value-changed events can be prevented by ensuring that the client-side value property is always initialized with an empty string when no custom value is provided.

@vursen vursen force-pushed the feat/big-decimal-field/remove-validated-event branch 2 times, most recently from eebf3c8 to 3b20298 Compare October 17, 2023 13:12
@vursen vursen marked this pull request as ready for review October 18, 2023 08:55
@vursen vursen assigned yuriy-fix and web-padawan and unassigned web-padawan and yuriy-fix Oct 18, 2023
@vursen vursen force-pushed the feat/big-decimal-field/remove-validated-event branch from 50cfbe1 to 74b2907 Compare October 18, 2023 10:02
@vursen vursen force-pushed the feat/big-decimal-field/remove-validated-event branch from 17e618b to 119d119 Compare October 18, 2023 10:35
@vursen vursen enabled auto-merge (squash) October 18, 2023 13:22
@vursen vursen merged commit de32b8d into main Oct 18, 2023
@vursen vursen deleted the feat/big-decimal-field/remove-validated-event branch October 18, 2023 13:27
@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 D 15 Code Smells

No Coverage information No Coverage information
0.5% 0.5% 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.

4 participants