Skip to content
This repository has been archived by the owner on Apr 6, 2022. It is now read-only.

Disable client-side validation on setRequiredIndicatorVisible() #77

Merged
merged 2 commits into from May 22, 2018

Conversation

denis-anisimov
Copy link

@denis-anisimov denis-anisimov commented May 21, 2018

Fixes #4077


This change is Reviewable

@caalador
Copy link
Contributor

Reviewed 15 of 16 files at r1.
Review status: 15 of 16 files reviewed at latest revision, all discussions resolved.


pom.xml, line 72 at r1 (raw file):

            <artifactId>hibernate-validator</artifactId>
            <version>6.0.1.Final</version>
        </dependency>

Where do we use hibernate? Also this is now a dependency both in dependencies and in runTests profile.


src/main/java/com/vaadin/flow/component/textfield/PasswordField.java, line 33 at r1 (raw file):

 * @author Vaadin Ltd.
 */
@JavaScript("frontend://textConnector.js")

Could these be run only when set requiredIndicatorVisible is set to true the first time?
A bit like we have in Dialog


src/main/java/com/vaadin/flow/component/textfield/RequiredValidaitonUtil.java, line 28 at r1 (raw file):

RequiredValidaitonUtil

Typo should be: RequiredValidationUtil


src/test/java/com/vaadin/flow/component/textfield/tests/AbstractRequiredValidationPage.java, line 52 at r1 (raw file):

setRequied

Typo: setRequired


src/test/java/com/vaadin/flow/component/textfield/tests/Entity.java, line 38 at r1 (raw file):

    }

}

new line


src/test/java/com/vaadin/flow/component/textfield/tests/MainView.java, line 58 at r1 (raw file):

        // b.forField(name)
        // .withValidator(a -> a != null && !a.isEmpty(), "eeeeeee")
        // .bind("name");

drop commented out code.


src/test/java/com/vaadin/flow/component/textfield/tests/MainView.java, line 79 at r1 (raw file):

    }

}

new line


Comments from Reviewable

@denis-anisimov
Copy link
Author

Review status: 15 of 16 files reviewed at latest revision, 7 unresolved discussions.


pom.xml, line 72 at r1 (raw file):

Previously, caalador wrote…

Where do we use hibernate? Also this is now a dependency both in dependencies and in runTests profile.

Right, should be inside runTests profile only.
It's not hibernate. It's validation implementation for JSR-303 provided by hibernate.
Done.


src/main/java/com/vaadin/flow/component/textfield/PasswordField.java, line 33 at r1 (raw file):

Previously, caalador wrote…

Could these be run only when set requiredIndicatorVisible is set to true the first time?
A bit like we have in Dialog

In reality the logic is executed only once on the client side. Because it checks whether it has been already executed.
I prefer do this on the client side instead of having a flag on the server side. It's more reliable since the server side is just a server side face for the client side.

But I think it may be done on the server side as well.
I will do that.


src/main/java/com/vaadin/flow/component/textfield/RequiredValidaitonUtil.java, line 28 at r1 (raw file):

Previously, caalador wrote…
RequiredValidaitonUtil

Typo should be: RequiredValidationUtil

Done.


src/test/java/com/vaadin/flow/component/textfield/tests/AbstractRequiredValidationPage.java, line 52 at r1 (raw file):

Previously, caalador wrote…
setRequied

Typo: setRequired

Done.


src/test/java/com/vaadin/flow/component/textfield/tests/Entity.java, line 38 at r1 (raw file):

Previously, caalador wrote…

new line

Done.


src/test/java/com/vaadin/flow/component/textfield/tests/MainView.java, line 58 at r1 (raw file):

Previously, caalador wrote…

drop commented out code.

The whole class is a leftover from the bug verification.
Removed.


src/test/java/com/vaadin/flow/component/textfield/tests/MainView.java, line 79 at r1 (raw file):

Previously, caalador wrote…

new line

Done.


Comments from Reviewable

@denis-anisimov
Copy link
Author

Review status: 15 of 16 files reviewed at latest revision, 7 unresolved discussions.


src/main/java/com/vaadin/flow/component/textfield/PasswordField.java, line 33 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

In reality the logic is executed only once on the client side. Because it checks whether it has been already executed.
I prefer do this on the client side instead of having a flag on the server side. It's more reliable since the server side is just a server side face for the client side.

But I think it may be done on the server side as well.
I will do that.

Ah, we are talking about different things. But anyway....


Comments from Reviewable

@denis-anisimov
Copy link
Author

Review status: 4 of 15 files reviewed at latest revision, 7 unresolved discussions.


src/main/java/com/vaadin/flow/component/textfield/PasswordField.java, line 33 at r1 (raw file):

Previously, denis-anisimov (Denis) wrote…

Ah, we are talking about different things. But anyway....

Done.


Comments from Reviewable

@caalador
Copy link
Contributor

Reviewed 1 of 17 files at r1, 11 of 13 files at r2.
Review status: all files reviewed at latest revision, all discussions resolved.


Comments from Reviewable

@denis-anisimov denis-anisimov merged commit 31da851 into master May 22, 2018
@denis-anisimov denis-anisimov deleted the 4077-disable-client-validation branch May 22, 2018 08:44
@pekam pekam added this to the 1.0.0.beta10 milestone May 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants