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: implement floating point number field #421

Merged
merged 14 commits into from
Dec 1, 2022
Merged

feat: implement floating point number field #421

merged 14 commits into from
Dec 1, 2022

Conversation

Skaiir
Copy link
Contributor

@Skaiir Skaiir commented Nov 22, 2022

Closes #285

Please try and break this one, I've been on it for a while so I might have developed some blindspots.

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Nov 22, 2022
@Skaiir Skaiir force-pushed the 285-decimal-number-rebase branch 2 times, most recently from 58f3395 to d6a86cc Compare November 23, 2022 08:42
@pinussilvestrus
Copy link
Contributor

@Skaiir can you please summarize the current requirements of the (decimal) number field in the respective issue? I know there was a back-and-forth via Slack, and we should make sure we track these things so we can get all the details later.

It would also help me to review as I'm not 100% up to date with the requirements (e.g. I saw labels changed and so on, missing the step field. I'm probably missing content here).

pinussilvestrus
pinussilvestrus suggested changes Nov 24, 2022
View reviewed changes
Copy link
Contributor

@pinussilvestrus pinussilvestrus left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Already left a couple of comments. Looks pretty promising 👍 I will have a deeper look at the number parsing in a second round.

packages/form-js-editor/package.json Show resolved Hide resolved
packages/form-js-editor/src/features/properties-panel/entries/NumberEntry.js Outdated Show resolved Hide resolved
packages/form-js-editor/src/features/properties-panel/entries/NumberEntry.js Outdated Show resolved Hide resolved
packages/form-js-viewer/src/render/components/form-fields/Number.js Outdated Show resolved Hide resolved
packages/form-js-viewer/src/render/components/FormComponent.js Show resolved Hide resolved
packages/form-js-viewer/test/spec/render/components/form-fields/Number.spec.js Outdated Show resolved Hide resolved
packages/form-js-viewer/test/spec/render/components/form-fields/util/NumberFieldUtil.spec.js Outdated Show resolved Hide resolved
packages/form-js-editor/src/features/properties-panel/PropertiesPanel.js Show resolved Hide resolved
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Nov 24, 2022
pinussilvestrus
pinussilvestrus reviewed Nov 28, 2022
View reviewed changes
Copy link
Contributor

@pinussilvestrus pinussilvestrus left a comment

Choose a reason for hiding this comment

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

A couple of more comments for the number field itself.

packages/form-js-viewer/test/spec/render/components/form-fields/Number.spec.js Show resolved Hide resolved
packages/form-js-viewer/src/render/components/form-fields/Number.js Show resolved Hide resolved
packages/form-js-viewer/src/render/components/form-fields/Number.js Outdated Show resolved Hide resolved
packages/form-js-viewer/src/render/components/form-fields/Number.js Show resolved Hide resolved
packages/form-js-viewer/src/render/components/form-fields/Number.js Outdated Show resolved Hide resolved
packages/form-js-viewer/src/render/components/form-fields/Number.js Outdated Show resolved Hide resolved
packages/form-js-viewer/test/spec/render/components/form-fields/Number.spec.js Show resolved Hide resolved
packages/form-js-viewer/src/core/Validator.js Outdated Show resolved Hide resolved
packages/form-js-viewer/src/render/hooks/useEffectDebugger.js Outdated Show resolved Hide resolved
packages/form-js-viewer/test/spec/render/components/form-fields/util/NumberFieldUtil.spec.js Outdated Show resolved Hide resolved
@Skaiir
feat: implement floating point number field
1be5f91 
Related to #285
@Skaiir
chore: test floating point number field
39ebc5c 
Closes #285
@Skaiir
chore: define big,js as an external dependency
2bcb35d 
Related to #285
@Skaiir
chore: decimal number field clarity and cleanups
b1bac93 
Related to #285
@Skaiir
chore: added stepper button test coverage
1588be7 
Related to #285
@Skaiir
fix: clear default value to null
4067996 
Related to #285
@Skaiir
chore: number properties panel test cov
3225d68 
Related to #285
@Skaiir
chore: decapitalized numberFieldUtil spec
e905388 
Related to #285
@Skaiir
fix: changed 'increment' to always validate >= 0
4753f85 
Related to #285
@Skaiir
fix: moved preactDebuggers in test folder
c33e16f 
Related to #285
@Skaiir
chore: added a11y tests to number field variants
9474ddf 
Related to #285
@Skaiir
chore: added test coverage to special input keys
eba7d24 
Closes #285
@Skaiir
chore: updated package lock
476758a 
Related to #285
pinussilvestrus
pinussilvestrus reviewed Dec 1, 2022
View reviewed changes
Copy link
Contributor

@pinussilvestrus pinussilvestrus left a comment

Choose a reason for hiding this comment

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

Thanks for the follow-up and the extensive test coverage 🚀 Looks good to me now, just one minor formatting comment. Afterward, let's merge 👍

/cc @vsgoulart as heads up that this includes now custom styling for number fields that would need to be handled in the tasklist makeover.

packages/form-js-viewer/src/core/Validator.js Outdated Show resolved Hide resolved
packages/form-js-viewer/src/render/components/FormComponent.js Show resolved Hide resolved
@Skaiir Skaiir merged commit 3cce402 into develop Dec 1, 2022
@Skaiir Skaiir deleted the 285-decimal-number-rebase branch December 1, 2022 09:47
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Dec 1, 2022
@vsgoulart
Copy link
Contributor

Thanks, @pinussilvestrus I was waiting for this PR, it will make it possible to completely style the number inputs

@philippfromme philippfromme mentioned this pull request Jan 5, 2023
Closed
41 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pinussilvestrus pinussilvestrus pinussilvestrus requested changes

@vsgoulart vsgoulart Awaiting requested review from vsgoulart

Assignees

@Skaiir Skaiir

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement decimal number Field
3 participants
@Skaiir @pinussilvestrus @vsgoulart