pierrejeambrun opened a new issue, #67647:
URL: https://github.com/apache/airflow/issues/67647
## Background
PR #67646 capped ``@chakra-ui/react`` at ``~3.34.0`` (patch only) because
the ``3.35`` bump pulled in ``@ark-ui/react >=5.36`` and triggered a
page-unresponsive bug after closing any Clear / Mark-as dialog without
confirming.
Per Ark UI's ``5.36.0`` release notes:
> **Dialog / Drawer**: Avoid setting inline ``pointer-events`` when modal,
> letting the dismissable layer manage it.
The cleanup moved from an inline DOM style (which disappeared on
unmount) to the dismissable layer's close-transition callback. Several
buttons in this codebase mount their dialog conditionally
(``{open ? <Dialog .../> : undefined}``), which unmounts the dialog
**before** the close transition runs and leaves the ``pointer-events: none``
lock stuck on ``document``.
## What needs doing
Unpin ``@chakra-ui/react`` so we can pick up 3.35+ and future Ark UI
fixes. To do so safely the conditional-mount sites must be rewritten
first.
## Steps
1. Pick one of the affected callers and rewrite it from:
```tsx
{open ? <SomeDialog ... open={open} onClose={onClose} /> : undefined}
```
to always-rendered:
```tsx
<SomeDialog ... open={open} onClose={onClose} />
```
The dialog's ``Dialog.Root`` already uses ``lazyMount``, so it stays
out of the DOM until the user actually opens it the first time. Once
open it stays mounted across open/close cycles, so Chakra/Ark gets
to run its close transition cleanly.
2. **Gate any expensive query inside the dialog with ``enabled: open``**
(or the equivalent boolean) so the now-always-mounted dialog
doesn't fire its dry-run on every render of the parent page.
3. **Reset the dialog's local form state on close** (e.g. ``note``,
segmented options) so a typed-but-cancelled note doesn't survive
until the next open. The cleanest pattern is to wrap ``onClose``:
```tsx
const handleClose = () => {
setNote(initialNote);
setSelectedOptions(defaultOptions);
onClose();
};
```
and pass ``handleClose`` to both ``Dialog.Root.onOpenChange`` **and**
the mutation's ``onSuccess(Confirm)`` callback.
4. Repeat for all sites listed below.
5. Bump ``@chakra-ui/react`` to ``^3.35.0`` (or latest) in
``airflow-core/src/airflow/ui/package.json`` and remove the
``Dependency upgrade caveats`` section in
``airflow-core/src/airflow/ui/CONTRIBUTING.md``.
## Affected files
Parent buttons / headers that need to drop the ``{open ? <Dialog/> :
undefined}`` wrapping:
- ``airflow-core/src/airflow/ui/src/components/Clear/Run/ClearRunButton.tsx``
-
``airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearTaskInstanceButton.tsx``
(three discriminator branches)
-
``airflow-core/src/airflow/ui/src/components/MarkAs/Run/MarkRunAsButton.tsx``
-
``airflow-core/src/airflow/ui/src/components/MarkAs/TaskInstance/MarkTaskInstanceAsButton.tsx``
- ``airflow-core/src/airflow/ui/src/pages/TaskInstance/Header.tsx``
Dialogs themselves (will need a ``handleClose`` and possibly an
``enabled: open`` on a dry-run query):
- ``airflow-core/src/airflow/ui/src/components/Clear/Run/ClearRunDialog.tsx``
-
``airflow-core/src/airflow/ui/src/components/Clear/TaskInstance/ClearTaskInstanceDialog.tsx``
-
``airflow-core/src/airflow/ui/src/components/MarkAs/Run/MarkRunAsDialog.tsx``
-
``airflow-core/src/airflow/ui/src/components/MarkAs/TaskInstance/MarkTaskInstanceAsDialog.tsx``
## How to verify
1. Open any Clear or Mark-as dialog from the Dag Runs list, the Task
Instances list, or the per-run / per-TI detail pages.
2. Close the dialog **without confirming** (X button, Escape, or click
the backdrop).
3. The rest of the page must still be clickable. Scroll alone isn't
enough — links, buttons, and table rows must respond.
4. Repeat after a confirmed action (Confirm + close on success) — same
expectation.
5. Reopen the same dialog: the note field should show the run/TI's
note value, **not** whatever you typed before cancelling.
## Reference
- The original bump: #66225
- The revert + cap: #67646
- An in-flight refactor of the conditional-mount sites:
#67645 — useful as a reference but it may or may not be merged when
you pick this up; double-check current ``main``.
## Notes
This is labelled ``good first issue`` because the mechanical change is
small and the diff pattern is the same in every file. Take it one
file at a time and let CI plus a quick manual click-test catch
mistakes.
--
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]
