-
Notifications
You must be signed in to change notification settings - Fork 14.1k
feat/TanStack-Form #18346
New issue
Have a question about this project? No Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “No 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? No Sign in to your account
feat/TanStack-Form #18346
Conversation
…indeterminate state refactor: remove mixed state handling and update related styles fix: update useCallback dependencies for better performance
… TextField, and SubmitButton with validation
…ld, SelectField, TextField, and SubmitButton with updated input sizes
…d tooltips; add OptionsField component
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This pull request introduces support for TanStack Form by updating existing hook dependencies and integrating new form components and demo scenarios. Key changes include:
- Refining useCallback and useEffect dependency arrays in the Completed component.
- Replacing legacy style props (e.g., className for sizing) with new "size" props in Input, InputNumber, and ParamItem components.
- Adding new TanStack Form components and demo setups to improve form handling consistency.
Reviewed Changes
Copilot reviewed 29 out of 30 changed files in this pull request and generated no comments.
Show a summary per file
File | Description |
---|---|
web/app/components/datasets/documents/detail/completed/index.tsx | Updated useCallback and useEffect dependencies and switched from "mixed" to "indeterminate" prop. |
web/app/components/datasets/create/step-two/inputs.tsx | Replaced custom className sizing with the "size" prop on InputNumber. |
web/app/components/base/param-item/index.tsx | Changed size property from 'sm' to 'regular' for consistent styling. |
web/app/components/base/input/index.tsx | Adjusted prop type omission for the size attribute to prevent conflicts. |
web/app/components/base/input-number/index.tsx | Updated valid size options and adjusted padding logic for buttons. |
web/app/components/base/form/** | Introduced TanStack Form contexts, hooks, and several demo form components. |
web/app/components/base/checkbox/index.tsx | Replaced "mixed" prop with "indeterminate" and updated styling for better clarity. |
Files not reviewed (1)
- web/app/components/base/checkbox/index.module.css: Language not supported
Comments suppressed due to low confidence (2)
web/app/components/datasets/documents/detail/completed/index.tsx:564
- [nitpick] The variable 'selectDefaultValue' returns both a string ('all') and a number (1 or 0) based on 'selectedStatus', which could lead to type ambiguity. Consider normalizing its return type to ensure consistent behavior in the SimpleSelect component.
const selectDefaultValue = useMemo(() => {
web/app/components/datasets/documents/detail/completed/index.tsx:378
- The useEffect hook uses 'resetList' without including it in the dependency array. If 'resetList' is not stable, this could lead to stale closures; consider adding 'resetList' to the dependency array or documenting why its omission is intentional.
useEffect(() => { resetList() // eslint-disable-next-line react-hooks/exhaustive-deps
…er components; enhance Jest configuration and setup
Summary
Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.
Tip
Close issue syntax:
Fixes #<issue number>
orResolves #<issue number>
, see documentation for more details.Screenshots
Checklist
Important
Please review the checklist below before submitting your pull request.
dev/reformat
(backend) andcd web && npx lint-staged
(frontend) to appease the lint gods