bxsx commented on code in PR #42494:
URL: https://github.com/apache/superset/pull/42494#discussion_r3694318680
##########
superset-frontend/src/features/alerts/AlertReportModal.tsx:
##########
@@ -2597,6 +2599,22 @@ const AlertReportModal:
FunctionComponent<AlertReportModalProps> = ({
</Checkbox>
</div>
)}
+ <div className="inline-container">
+ <Checkbox
+ data-test="include-cta"
+ checked={currentAlert?.include_cta !== false}
+ onChange={(e: CheckboxChangeEvent) =>
+ updateAlertState('include_cta', e.target.checked)
+ }
Review Comment:
The hydration race is real but modal-wide, not specific to this control: the
`[resource]` effect replaces `currentAlert` wholesale, so every field in this
dialog has the same window — including the `force_screenshot` checkbox directly
above this one (`setForceScreenshot(resource.force_screenshot)`), the name
field, report format, and notification settings. The new checkbox deliberately
follows the same hydration pattern as its sibling per-schedule settings. It's
also not silently persisted: the control is fully controlled by `currentAlert`,
so an early click visibly reverts to the fetched value once hydration
completes. Gating the form on `loading` (or merging dirty fields into the
fetched state) would be a worthwhile UX improvement, but it's a modal-wide
change affecting a dozen fields — better done in its own PR than bolted onto
one checkbox.
--
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]
