ENG-3549: Migrate SystemInformationForm + DictSuggestion + VendorSelector to antd#8235
Merged
Conversation
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
|
gilluminate
force-pushed
the
gill/eng-3549/system-info-form-antd
branch
from
c1c718d to
09b76cb
Compare
Contributor
|
Running into some issues here:
|
jpople
reviewed
May 20, 2026
gilluminate
force-pushed
the
gill/eng-3549/system-info-form-antd
branch
from
93975c3 to
2ba52b8
Compare
Contributor
Author
for the life of me I cannot reproduce this, might be a stale build or something. At any rate, Claude seems to know the safeguard for it.
You are right, this was pre-existing. I'll create a follow-up bug ticket.
looks like I missed validation in 2 places. Good catch!
Is it supposed to?
This appears to be pre-existing as well. Disabling the button is an easy interim fix I'll do here and address this along with the GVL locking. Seems related. |
…ctor to antd Replaces the Formik/Chakra-based System Information form (30 fields across 5 sections) with a single antd Form. Swaps VendorSelector for the already-existing antd-native VendorSelectorAnt, rewrites DictSuggestionInputs on top of Form.useFormInstance/useWatch, ports the async system-name uniqueness check to a Form.Item validator, and converts SystemFormInputGroup to an antd Card. CustomFieldsList and FormGuard are also migrated since SystemInformationForm was their only remaining Formik consumer.
- Merge initialValues with antd onFinish values so unregistered fields (privacy_declarations, system_type, etc.) make it into the POST payload - Drop required-validation rules on conditional fields (reason_for_exemption, legal_basis_for_profiling/transfers, dpa_location) since the original Yup schema only validated name + privacy_policy. Compass-driven autofill flips some toggles on which would otherwise block submit on empty conditional fields the user never edited - Push customFieldValues into the form via useEffect once they finish loading async (key-based remount was broken because useCustomFields isLoading flips on create flow too) - Add controlled-select-* testids on Select components to match the existing test contract
When picking a "Create new system" option (or any vendor with no
vendor_id), VendorSelector dispatches setSuggestions("hiding") even though
"showing" was never dispatched. The DictSuggestion hook then restored every
field from an uncaptured preSuggestionRef → wrote undefined into
processes_personal_data → conditional Administrative / Cookie sections
unmounted → cypress couldn't find input-legal_name.
- Guard the restore branch with a hasCapturedRef so we only restore when
we actually snapshotted a pre-suggestion value
- Default Form.useWatch reads to initialValues so transient undefineds
during the show/hide lifecycle (and on the very first render) don't
blink conditional sections out
Fixes the CI-only "does not lock editing for a non-GVL vendor" failure
which timed out finding input-legal_name on a 1280x720 viewport.
Two validation regressions from the migration:
1. The system-name uniqueness rule was attached to the hidden Form.Item
inside VendorSelector. The validator ran and rejected submit, but the
error message had nowhere to render. Wrap the visible Select / Input
in a Form.Item shouldUpdate render-prop that reads form.getFieldError("name")
and passes it through validateStatus + help so the error renders on the
actual field (matches main).
2. The original Yup schema had `privacy_policy: Yup.string().min(1).url()`.
I dropped it during the migration. Add a `rules` prop to
DictSuggestionTextInput and wire `{ type: "url" }` for the privacy
policy field.
Also makes handleSubmit's privacy_declarations.length check defensive
with optional chaining so edit-flow systems missing the field don't blow
up the handler before the merge with initialValues lands.
Previously the Compass button sat as a sibling of the typeahead Form.Item in a Flex with align="flex-end". When the name uniqueness error appeared below the input, the Form.Item grew taller and flex-end pushed the button down to the bottom of the error. Move the button inside the visible Form.Item, next to the Select/Input in a center-aligned row, so the help text renders below the entire row and the button stays put.
1. useSystemFormTabs.handleSuccess passed `message` to notification.success, which is deprecated in antd v6 — renamed to `title`. 2. After picking a real option in VendorSelector, searchParam stayed populated, so optionsWithCustom kept the synthetic "Create new system \"X\"…" entry. Its value matched the now-selected name (also "X"), but its label differed, triggering antd's "label of value is not same as label in Select options" warning. Reset searchParam in handleChange so the synthetic option drops out after selection.
The old <fieldset disabled> wrapper relied on the browser-level disabled mechanism, which native inputs respect but antd components mostly ignore visually — viewers saw a fully editable-looking form, could change values and click an enabled Save button that silently failed. - Replace the fieldset with antd Form's `disabled` prop. That propagates through Form context to every Form.Item-bound control (Selects, Inputs, Switches, InputNumber, DictSuggestion* family), disabling them consistently with proper antd disabled styling. - VendorSelector renders the visible Select/Input outside of Form.Item's auto-binding (because labelInValue + manual setFieldsValue), so it doesn't pick up the Form-context disabled. Plumb an explicit `disabled` prop through VendorSelectorProps and OR it into nameFieldLockedForGVL plus the Compass button's disabled state.
CustomSelect isn't the culprit — it correctly forwards props to the inner
antd Select. The bug is React's normal prop precedence: passing
\`disabled={false}\` explicitly to an antd input overrides the
DisabledContext set by an ancestor \`<Form disabled>\`.
Most Form.Item-bound inputs in SystemInformationForm carry conditional
disables like \`disabled={lockedForGVL}\`, etc. When those expressions
evaluate to false (the common case in normal view), they hard-set
\`disabled={false}\` and viewers' Form-level disabled never wins.
Convert every conditional \`disabled={someBool}\` to \`disabled={someBool
|| undefined}\` so a false local condition defers to Form context.
This reverts commit 85fcb9b.
This reverts commit 912e081.
gilluminate
force-pushed
the
gill/eng-3549/system-info-form-antd
branch
from
3c2878d to
fcf8003
Compare
jpople
approved these changes
May 20, 2026
Contributor
jpople
left a comment
jpople
left a comment
There was a problem hiding this comment.
Thanks for the changes, everything looks to be working now and the submit issue is cleared up-- no idea what was going on there. Approving with one last tiny nit, can we adjust the spacing below the name/vendor input here so it's even?
Contributor
Author
|
@jpople oh yeah! I keep meaning to do that and keep getting distracted. Thank you for the reminder! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ticket ENG-3549
Description Of Changes
Part of ENG-3187 (System inventory + add-system forms → Ant). This is the biggest single chunk: the main
SystemInformationForm(30 fields across 5 sections), theVendorSelectortypeahead, theDictSuggestionInputsfamily, and theSystemFormInputGrouplayout wrapper. After this lands, the system-information surface is fully off Formik/Chakra.Two prior migrations made this tractable:
AddNewSystemModalandAddVendorsince the ENG-3187 prep work. This PR finally retires the Formik original.*FormItem-per-field pattern and thekey={entity?.id ?? "create"}re-init pattern.CustomFieldsListandFormGuardwere migrated alongside becauseSystemInformationFormwas their only remaining Formik consumer — keeping them on Formik would have left a dangling dependency.Code Changes
clients/admin-ui/src/features/system/SystemInformationForm.tsx— full rewrite.<Formik>→ antd<Form>withForm.useForm, all 30 fields ported toForm.Item+ native antd components (Input,Switch,Select,InputNumber). Yup async name-uniqueness check ported to aForm.Itemvalidatorrule. Conditional sections (processes_personal_data,exempt_from_privacy_regulations,uses_profiling,does_international_transfers,requires_data_protection_assessments) now driven byForm.useWatch.enableReinitializereplaced bykey={passedInSystem?.fides_key ?? "create"}on the<Form>element.clients/admin-ui/src/features/system/dictionary-form/DictSuggestionInputs.tsx— rewroteuseDictSuggestionagainstForm.useFormInstance/useWatch/setFieldValue; replaced the 4 field components (Chakra → antdInput,Input.TextArea,Switch,InputNumber); dropped the Formik blur/setTimeout(300)workaround entirely.clients/admin-ui/src/features/system/dictionary-form/DictSuggestionInputs.module.scss— new SCSS module for the--fidesui-complimentary-500"suggested" coloring (per fidesui color-token convention).clients/admin-ui/src/features/system/SystemFormInputGroup.tsx— Chakra layout primitives → antdCard.clients/admin-ui/src/features/system/VendorSelector.tsx— deleted.VendorSelectorAntis now the sole vendor selector. The stale "when those migrate, swap their imports here" comment onVendorSelectorAntwas also cleaned up.clients/admin-ui/src/features/common/custom-fields/CustomFieldsList.tsx— Formik<Field>→ antdForm.Item. Existing consumers (system form, plus the already-migrated privacy declaration forms viaPrivacyDeclarationCustomFields) continue to work.clients/admin-ui/src/features/common/hooks/useIsAnyFormDirty.tsx—FormGuardrewritten againstForm.useFormInstance+Form.useWatch([], form)to keepisFieldsTouched()reactive across all field updates.clients/admin-ui/cypress/e2e/systems.cy.ts— updated the duplicate-name assertion to use the existingcy.getAntFormError("name")helper (antd renders errors in.ant-form-item-explain-error, not the olddata-testid="error-name").The submit button uses a deep-compare dirty check (
isEqual(form.getFieldsValue(true), initialValues)) per the antd-migration playbook's escape hatch —VendorSelectorAntand the dict-suggestion fields commit values viasetFieldsValue, which doesn't triggerisFieldsTouched.Steps to Confirm
complimentary-500"suggested" color, and Save persists name + vendor_id.processes_personal_dataoff → "Cookie properties" / "Administrative properties" sections collapse. Toggle back on → they re-appear with preserved values.uses_profiling→ "Legal basis for profiling" appears below it; toggle off → it disappears. Repeat fordoes_international_transfers(→ legal basis for transfer),requires_data_protection_assessments(→ DPIA/DPA location),exempt_from_privacy_regulations(→ reason for exemption).SYSTEM_UPDATE, all fields render disabled and the Save button is hidden.Pre-Merge Checklist
CHANGELOG.mdupdated