ENG-3179: Migrate properties form to Ant Design#7880
Conversation
Replace Chakra UI components (Box, Flex, FormSection) and Formik form state with Ant Design Form, Card, Select, and Input. Adds proper form validation via antd rules, loading state handling, and disabled input theme tokens for read-only fields. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
4ffdc4f to
f9ead53
Compare
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
f9ead53 to
69f8d0b
Compare
There was a problem hiding this comment.
Code Review: Properties Form Antd Migration (#7880)
This PR migrates PropertyForm from Chakra/Formik to Ant Design and also cleans up read-only input styling in fidesui. The overall approach is solid and the component structure is clean. A couple of issues are worth addressing before merge.
🔴 Issues to Address
1. Experience configs dropdown uses stale table pagination state
(PropertyForm.tsx:45-46)
The form queries experience configs using selectPage/selectPageSize from the privacy-experience Redux slice — whatever page the user last visited in the experience table. This causes the multi-select to silently show only a subset of available experiences. A user on edit mode could see an incomplete list and accidentally deselect experiences that simply aren't on the cached page. The query should use a fixed { page: 1, size: 200 } (or similar) decoupled from the table state.
2. form.isFieldsTouched() blocks Save on the edit page
(PropertyForm.tsx:211)
setFieldsValue (called on load) does not mark fields as touched, so the Save button stays disabled when the form loads with an existing property's data. A user who wants to save without changes (e.g. after an upstream edit) cannot. The previous Formik dirty check compared values against initialValues and didn't have this problem. Consider a value-diff dirty check or simply rely on !submittable alone.
🟡 Suggestions
3. Verify Space.Addon is a valid fidesui export
(PropertyForm.tsx:186-188)
Space.Addon is not a standard antd component. If it's a custom fidesui export, the .ant-space-addon CSS class targeted in global.scss needs to match what it renders. Worth a quick visual check that the clipboard button in the Property ID row looks correct.
4. Typo: --ant-input-hove-shadow → --ant-input-hover-shadow
(global.scss:129)
Missing r — the hover shadow suppression on read-only inputs has never taken effect. This was pre-existing but easy to fix while touching this area.
✅ What's Good
- Clean removal of Chakra/Formik dependencies throughout the form and its pages.
isLoadingprop +Spinfallback gives a good loading state UX.getValueProps/getValueFromEventpattern for the experiencesSelectis the correct antd way to bridge array-of-objects ↔ array-of-IDs.- Unifying read-only input styles to reference the disabled token variables is a nice consistency improvement in
global.scss. - The
default-theme.tsreordering (movingSelectafterPopover) is a minor cleanup, no functional change.
🔬 Codegraph: unavailable
💡 Write /code-review in a comment to re-run this review.
- Decouple experience configs query from table pagination state - Fix typo in read-only input hover shadow CSS variable Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Ticket ENG-3179
Description Of Changes
Migrates the properties form (create + edit) from Chakra UI components and Formik form state to Ant Design Form, Card, Select, and Input. Adds proper form validation via antd rules, a loading spinner for the edit page, and disabled input theme tokens so read-only fields match the disabled style.
Code Changes
ChakraBox,ChakraFlex,FormSection,CustomTextInput,ControlledSelect,ScrollableListwith AntForm,Card,Flex,Input,Select<Formik>,useFormikContext,enableReinitialize) with antdForm.useForm,Form.useWatch,Form.ItemrulesisLoadingprop toPropertyFormfor edit page loading statecolorBorderDisabled,colorTextDisabled,colorBgContainerDisabled) to fidesui default themeSpace.Addon)PropertyFormSteps to Confirm
Pre-Merge Checklist
CHANGELOG.mdupdatedmaindowngrade()migration is correct and works