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.

Sign up for GitHub

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

Jump to bottom

fix: disable client side validation #973

Merged
merged 9 commits into from
May 18, 2021
Merged

fix: disable client side validation #973

merged 9 commits into from
May 18, 2021

Conversation

sissbruecker
Copy link
Contributor

@sissbruecker sissbruecker commented May 10, 2021 •
edited

Description

The original issue occurs because the select web component triggers its own client-side validation in several places (specifically to the issue, when the dropdown is closed), which then overwrites the validation state set by the server. Investigating this also lead to another issue that changing the validation state on the client through JS / dev tools also changed the validation state on the server, which should not be allowed.

This change fixes that by:

  • overwriting the client-side validation method with a no-op, as is done for other field components
  • disabling synchronization of the client side validation state to the server
  • semi-related change: fixes the initial value of the client side component to be an empty string instead of the string null which initially lead the client side component to pass validation, because the value was not empty

Fixes vaadin/vaadin-select#265

Type of change

  • Bugfix

tulioag
tulioag suggested changes May 11, 2021
View reviewed changes
Copy link
Contributor

@tulioag tulioag left a comment

Choose a reason for hiding this comment

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

Suggestions:

  1. Test detaching and reattaching the select element and see if everything still works as expected
  2. Test a select on a Grid

...ct-flow-parent/vaadin-select-flow/src/main/java/com/vaadin/flow/component/select/Select.java Show resolved Hide resolved
@vlukashov vlukashov mentioned this pull request May 11, 2021
Closed
@sissbruecker sissbruecker requested a review from tulioag May 17, 2021 07:52
@sissbruecker sissbruecker force-pushed the fix/select-disable-client-side-validation branch from 1d88aa6 to 2b13b9d Compare May 18, 2021 06:31
@sissbruecker sissbruecker enabled auto-merge (squash) May 18, 2021 06:31
@sissbruecker sissbruecker merged commit 14676de into master May 18, 2021
@sissbruecker sissbruecker deleted the fix/select-disable-client-side-validation branch May 18, 2021 06:38
@vaadin-bot
Copy link
Collaborator

SonarQube analysis reported 16 issues

  • CRITICAL 5 critical
  • MAJOR 1 major
  • MINOR 8 minor
  • INFO 2 info

Top 10 issues

  1. CRITICAL Select.java#L206: Remove usage of generic wildcard type. rule
  2. CRITICAL Select.java#L473: Remove usage of generic wildcard type. rule
  3. CRITICAL GeneratedVaadinSelect.java#L275: Define a constant instead of duplicating this literal "opened" 3 times. rule
  4. CRITICAL GeneratedVaadinSelect.java#L404: Define a constant instead of duplicating this literal "invalid" 3 times. rule
  5. CRITICAL GeneratedVaadinSelect.java#L688: Define a constant instead of duplicating this literal "value" 4 times. rule
  6. MAJOR Select.java#L987: Rename "dataProvider" which hides the field declared at line 88. rule
  7. MINOR Select.java#L479: Remove this use of "setDataProvider"; it is deprecated. rule
  8. MINOR Select.java#L504: Remove this use of "setDataProvider"; it is deprecated. rule
  9. MINOR Select.java#L553: Remove this method to simply inherit it. rule
  10. MINOR GeneratedVaadinSelect.java#L684: Move this constructor to comply with Java Code Conventions. rule

vaadin-bot pushed a commit that referenced this pull request May 18, 2021
@sissbruecker @tulioag @vaadin-bot
fix: disable client side validation (#973)
1cf31f8 
* fix: disable client validation

* Simplify override client validation IT

* Simplify override client validation IT

* Extract FieldValidationUtil

* Ensure correct inital client-side value

* Fix formatting

* Add test for detach/reattach scenario

* Add test for select in grid usage

* Use project version variable for Maven dependency

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>
@vaadin-bot
Copy link
Collaborator

Hi @sissbruecker , this commit cannot be picked to 19.0 by this bot, can you take a look and pick it manually?

@vaadin-bot
Copy link
Collaborator

Hi @sissbruecker , this commit cannot be picked to 14.7 by this bot, can you take a look and pick it manually?

@vaadin-bot
Copy link
Collaborator

Hi @sissbruecker , this commit cannot be picked to 14.6 by this bot, can you take a look and pick it manually?

sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973)
83bee9c 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973)
914478b 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973)
3cbf68b 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973)
6721965 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973)
f82bacc 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973) (#989)
ef48d7f 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973) (#988)
91200ee 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@sissbruecker
CP: disable client side validation (#973) (#986)
69923d1 
Fixes: vaadin/vaadin-select#265
Warranty: Fixes client-side select overriding validation state from server

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

(cherry picked from commit 14676de)
sissbruecker added a commit that referenced this pull request May 18, 2021
@vaadin-bot @sissbruecker @tulioag
fix: disable client side validation (#973) (#982)
b7d4020 
* fix: disable client validation

* Simplify override client validation IT

* Simplify override client validation IT

* Extract FieldValidationUtil

* Ensure correct inital client-side value

* Fix formatting

* Add test for detach/reattach scenario

* Add test for select in grid usage

* Use project version variable for Maven dependency

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>

Co-authored-by: Sascha Ißbrücker <sissbruecker@vaadin.com>
Co-authored-by: Tulio Garcia <28783969+tulioag@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tulioag tulioag tulioag approved these changes

@yuriy-fix yuriy-fix Awaiting requested review from yuriy-fix

Assignees
No one assigned
Labels
cherry-picked-20.0 need to pick manually 14.6 need to pick manually 14.7 need to pick manually 19.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Not selecting a value clears validator error
3 participants
@sissbruecker @vaadin-bot @tulioag