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

feat: integrate client-side validation into TimePicker #3515

Merged
merged 25 commits into from
Sep 1, 2022
Merged

feat: integrate client-side validation into TimePicker #3515

merged 25 commits into from
Sep 1, 2022

Conversation

DiegoCardoso
Copy link
Contributor

@DiegoCardoso DiegoCardoso commented Jul 26, 2022 •
edited by vursen
Loading

Description

Implement the addValidationStatusChangeListener to trigger Binder validation when validated event is received from the web component.

Depends on

Part of vaadin/platform#3066

Type of change

  • Bugfix
  • Feature

@vursen vursen force-pushed the feat/time-picker/validated-event branch from 3ba64f4 to 95cc604 Compare August 9, 2022 13:33
@vursen vursen mentioned this pull request Aug 10, 2022
Merged
7 tasks
@vursen
Copy link
Contributor

vursen commented Aug 10, 2022

As was discovered, the time-picker web component prevents the change event on the input element until it loses focus, so it is possible to commit a non-empty value on Enter but have the hasInputValue state not updated.

This is why integration tests are failing.

@vursen vursen mentioned this pull request Aug 11, 2022
Merged
9 tasks
@vursen vursen force-pushed the feat/time-picker/validated-event branch from 0069e06 to ff60c52 Compare August 11, 2022 13:53
@vursen vursen mentioned this pull request Aug 12, 2022
Merged
2 tasks
@vursen vursen force-pushed the feat/time-picker/validated-event branch from 8627ded to 166527a Compare August 14, 2022 05:48
@vursen vursen mentioned this pull request Aug 15, 2022
Merged
7 tasks
@vursen vursen force-pushed the feat/time-picker/validated-event branch from 64f077a to 658c925 Compare August 15, 2022 10:35
@vursen vursen mentioned this pull request Aug 15, 2022
Closed
@vursen vursen force-pushed the feat/time-picker/validated-event branch 3 times, most recently from 76e9836 to 9d81d02 Compare August 15, 2022 18:08
@vursen vursen marked this pull request as ready for review August 15, 2022 18:08
@vursen vursen force-pushed the feat/time-picker/validated-event branch from 9d81d02 to 30e9b9a Compare August 28, 2022 08:29
@vursen vursen requested a review from yuriy-fix August 29, 2022 08:33
@vursen vursen force-pushed the feat/time-picker/validated-event branch from d0bd316 to a4ceb3d Compare August 30, 2022 12:33
yuriy-fix
yuriy-fix reviewed Aug 31, 2022
View reviewed changes
...sts/src/test/java/com/vaadin/flow/component/timepicker/tests/TimePickerValidationPageIT.java
import com.vaadin.tests.AbstractValidationTest;
import com.vaadin.flow.testutil.TestPath;
import org.junit.Assert;
import org.junit.Test;
import org.openqa.selenium.WebElement;

/**
* Integration tests for {@link TimePicker} validation.
*/
@TestPath("vaadin-time-picker/time-picker-validation")
public class TimePickerValidationPageIT extends AbstractValidationTest {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need TimePickerValidationPageIT and corresponding view for it?

Copy link
Contributor

@vursen vursen Aug 31, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, it verifies that the HasValidation interface is implemented for TimePicker. Improved the description. As we discussed internally, these integration tests can be rewritten into unit tests, but we will do that separately.

@vursen vursen enabled auto-merge (squash) September 1, 2022 13:00
@vursen vursen disabled auto-merge September 1, 2022 13:10
@vursen vursen force-pushed the feat/time-picker/validated-event branch from 50dedfc to 3c8dd15 Compare September 1, 2022 13:15
@vursen vursen requested a review from yuriy-fix September 1, 2022 13:31
@vursen vursen enabled auto-merge (squash) September 1, 2022 14:14
@sonarqubecloud
Copy link

sonarqubecloud bot commented Sep 1, 2022

SonarCloud Quality Gate failed.    Quality Gate failed

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

No Coverage information No Coverage information
0.0% 0.0% Duplication

@vursen vursen merged commit fcbac5e into master Sep 1, 2022
@vursen vursen deleted the feat/time-picker/validated-event branch September 1, 2022 14:22
@vaadin-bot
Copy link
Collaborator

This ticket/PR has been released with Vaadin 23.2.0.

@mukherjeesudebi mukherjeesudebi mentioned this pull request Jul 14, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@vursen vursen vursen left review comments

@yuriy-fix yuriy-fix yuriy-fix approved these changes

Assignees
No one assigned
Labels
Released with Vaadin 23.2.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@DiegoCardoso @vursen @vaadin-bot @yuriy-fix