gabotorresruiz opened a new pull request, #44356:
URL: https://github.com/apache/superset/pull/44356
### SUMMARY
On the first open of the dashboard properties modal, Save/Apply is disabled
and its tooltip reads "General information: Dashboard name is required", even
though the name input is populated.
Closing and reopening the modal, or editing the name, makes the button
enabled.
**Mechanism.** The `basic` section validator reads the title through
`form.getFieldsValue()`.
Without arguments, that call only returns fields that are already registered
with the form.
On the first open the title `FormItem` registers after the data-ready effect
has already run `validateSection('basic')`, so `values.title` is `undefined`,
the section is flagged with an error, and `hasErrors` disables the primary
button.
Nothing re-validates the section until the user edits a field, which is why
the state sticks.
On a second open the field is already registered, so the same code path
passes.
**Fix.** Read the title from the form store with
`form.getFieldValue('title')`, which does not depend on field registration.
This is the same read `BasicInfoSection` already uses for its inline error,
so the section label and the field can no longer disagree.
No change to the always-enabled Save/Apply contract of `StandardModal`.
**Test.** `PropertiesModal.firstOpen.test.tsx` mounts `BasicInfoSection` one
render after its parent, reproducing the late registration, and asserts Apply
is enabled with the name populated.
It fails on master and passes with this change.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before: first open shows Apply disabled with the tooltip "General
information: Dashboard name is required" while the name input reads the
dashboard title.
After: Apply is enabled on the first open; clearing the name still disables
it with the error, and typing a name re-enables it.
### TESTING INSTRUCTIONS
1. Open any dashboard, enter edit mode, and pick "Edit properties" from the
actions menu.
2. Before this change, Apply is disabled and hovering it shows the
"Dashboard name is required" tooltip; with this change it is enabled.
3. Clear the name: Apply becomes disabled with the error. Type a name: it is
enabled again.
4. `npx jest src/dashboard/components/PropertiesModal`
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [x] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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]
