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(TypeScript): binder should allow typing decimal numbers #9604

Merged
merged 5 commits into from
Dec 16, 2020
Merged

fix(TypeScript): binder should allow typing decimal numbers #9604

merged 5 commits into from
Dec 16, 2020

Conversation

platosha
Copy link
Contributor

@platosha platosha commented Dec 8, 2020

Fixes #9521
Fixes #9522

@platosha platosha added the hilla Issues related to Hilla label Dec 8, 2020
haijian-vaadin
haijian-vaadin suggested changes Dec 10, 2020
View reviewed changes
Copy link
Contributor

@haijian-vaadin haijian-vaadin left a comment

Choose a reason for hiding this comment

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

Also, could we have a test for the fix?

flow-client/src/main/resources/META-INF/resources/frontend/form/Validators.ts Outdated Show resolved Hide resolved
@platosha platosha force-pushed the ap/fix/forms/number-binding branch 2 times, most recently from 69335e5 to 2740b6d Compare December 10, 2020 19:28
@platosha platosha changed the title WIP: fix(TypeScript): binder should allow typing decimal numbers fix(TypeScript): binder should allow typing decimal numbers Dec 11, 2020
@platosha platosha marked this pull request as ready for review December 11, 2020 10:27
@platosha
Copy link
Contributor Author

@haijian-vaadin added tests, there is one ensuring that the value is not reset from the binder while typing.

@haijian-vaadin
Copy link
Contributor

It looks good now, just found another issue. If I type 12,5 or 12,kjsdf into the field, I don't get a validation error, the data is saved as 12. Could this be fixed in this PR, so should it be in another ticket?

@platosha platosha force-pushed the ap/fix/forms/number-binding branch 2 times, most recently from 323bc15 to 5382d7d Compare December 14, 2020 13:08
@platosha
Copy link
Contributor Author

@haijian-vaadin thanks for pointing out, fixed here also

@haijian-vaadin
Copy link
Contributor

There are some lint errors for Models.ts file

@platosha
Copy link
Contributor Author

@haijian-vaadin oh, I always forget those, fixed 😅

haijian-vaadin
haijian-vaadin suggested changes Dec 14, 2020
View reviewed changes
Copy link
Contributor

@haijian-vaadin haijian-vaadin left a comment

Choose a reason for hiding this comment

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

It works! Good job! Found some minors things that could be improved.

flow-client/src/test/frontend/form/FieldTests.ts Outdated Show resolved Hide resolved
flow-client/src/test/frontend/form/ValidatorsTests.ts Show resolved Hide resolved
flow-client/src/test/frontend/form/FieldTests.ts Outdated Show resolved Hide resolved
@platosha platosha merged commit f946d33 into master Dec 16, 2020
@platosha platosha deleted the ap/fix/forms/number-binding branch December 16, 2020 10:44
platosha added a commit that referenced this pull request Dec 16, 2020
@platosha
fix(TypeScript): binder should allow typing decimal numbers (#9604)
0486772 
* WIP: fix(TypeScript): binder should allow typing decimal numbers

Fixes #9521
Fixes #9522

* fix linter error

* fix: disallow incorrect decimal separator (,) and extra characters

* fix linter errors

* cleaup: remove debugger

(cherry picked from commit f946d33)
platosha added a commit that referenced this pull request Dec 16, 2020
@platosha
fix(TypeScript): binder should allow typing decimal numbers (#9604)
f029101 
* WIP: fix(TypeScript): binder should allow typing decimal numbers

Fixes #9521
Fixes #9522

* fix linter error

* fix: disallow incorrect decimal separator (,) and extra characters

* fix linter errors

* cleaup: remove debugger

(cherry picked from commit f946d33)
platosha added a commit that referenced this pull request Dec 17, 2020
@platosha
fix(TypeScript): binder should allow typing decimal numbers (#9604)
61ecab0 
* WIP: fix(TypeScript): binder should allow typing decimal numbers

Fixes #9521
Fixes #9522

* fix linter error

* fix: disallow incorrect decimal separator (,) and extra characters

* fix linter errors

* cleaup: remove debugger

(cherry picked from commit f946d33)
platosha added a commit that referenced this pull request Dec 17, 2020
@platosha
fix(TypeScript): binder should allow typing decimal numbers (#9604)
58cb2cd 
* WIP: fix(TypeScript): binder should allow typing decimal numbers

Fixes #9521
Fixes #9522

* fix linter error

* fix: disallow incorrect decimal separator (,) and extra characters

* fix linter errors

* cleaup: remove debugger

(cherry picked from commit f946d33)
haijian-vaadin pushed a commit that referenced this pull request Dec 18, 2020
@platosha @haijian-vaadin
fix(TypeScript): binder should allow typing decimal numbers (#9604)
e74c8da 
* WIP: fix(TypeScript): binder should allow typing decimal numbers

Fixes #9521
Fixes #9522

* fix linter error

* fix: disallow incorrect decimal separator (,) and extra characters

* fix linter errors

* cleaup: remove debugger

(cherry picked from commit f946d33)
haijian-vaadin pushed a commit that referenced this pull request Dec 18, 2020
@platosha @haijian-vaadin
fix(TypeScript): binder should allow typing decimal numbers (#9604)
f4413cf 
* WIP: fix(TypeScript): binder should allow typing decimal numbers

Fixes #9521
Fixes #9522

* fix linter error

* fix: disallow incorrect decimal separator (,) and extra characters

* fix linter errors

* cleaup: remove debugger

(cherry picked from commit f946d33)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@haijian-vaadin haijian-vaadin haijian-vaadin approved these changes

Assignees
No one assigned
Labels
hilla Issues related to Hilla +0.0.1
Projects
None yet
Milestone
No milestone
3 participants
@platosha @haijian-vaadin @vaadin-bot