EnxDev commented on PR #41399:
URL: https://github.com/apache/superset/pull/41399#issuecomment-4842541573

   ## EnxDev's Review Agent — apache/superset#41399 · HEAD aec4b14
   **lgtm** — correct one-line fix for a real AntD-v5 controlled-field 
regression, shipped with a red/green regression test.
   
   The root-cause analysis is accurate. With `name="allowed-domains"`, AntD's 
`Form.Item` clones the child and injects its own `value`/`onChange` from the 
form store, so the explicit `value={allowedDomains}` is ignored and the 
post-GET `setAllowedDomains(...)` never reaches the rendered input. Replacing 
`name` with `htmlFor` demotes `FormItem` to a label/layout wrapper and restores 
the normal controlled `<Input>`.
   
   Verified no collateral damage from dropping the field:
   - The component is fully controlled by React state. Save/enable read 
`stringToList(allowedDomains)` and `isDirty` compares React state — there is no 
`Form.useForm()` instance, no `form.submit()`, no `getFieldsValue()`. `<Form>` 
is a pure layout wrapper, so removing `name` breaks nothing in save/validate. 
(`index.tsx:197`)
   - `htmlFor="allowed-domains"` ↔ `id="allowed-domains"` keeps the label↔input 
association, so accessibility and the test's `findByRole('textbox', { name: 
/Allowed Domains/i })` query still resolve. (`index.tsx:200`, `index.tsx:211`)
   
   ### 🙌 Praise
   - `EmbeddedModal.test.tsx:142` — proper regression guard: asserts the input 
`.value` reflects the API response (`example.com`), which fails on `master` and 
passes here. codecov shows the changed line covered.
   
   <!-- enxdev-review-agent:aec4b14 -->
   _Reviewed by EnxDev's Review Agent — @EnxDev · HEAD aec4b14._
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to