parkhojeong commented on code in PR #66717:
URL: https://github.com/apache/airflow/pull/66717#discussion_r3221156242
##########
airflow-core/src/airflow/ui/src/queries/useTrigger.ts:
##########
@@ -48,9 +48,15 @@ export const useTrigger = ({ dagId, onSuccessConfirm }: {
dagId: string; onSucce
});
onSuccessConfirm();
- // Only redirect if we're already on the dag page
+ // Only redirect if we're already on the dag page.
+ // Preserve search params so layout state (Grid limit, filters) survives
the navigation.
if (selectedDagId === dagRun.dag_id) {
- void
Promise.resolve(navigate(`/dags/${dagRun.dag_id}/runs/${dagRun.dag_run_id}`));
+ void Promise.resolve(
+ navigate({
+ pathname: `/dags/${dagRun.dag_id}/runs/${dagRun.dag_run_id}`,
+ search: globalThis.location.search,
+ }),
Review Comment:
1. Query Param Scope
I think preserving the entire search string here is a bit too broad. Some
of these params are not just passive UI state; for example `run_after_lte`
defines the Grid's run window in `DetailsLayout`.
If a user is currently looking at an older run/window and triggers a new
Dag run, this redirect will carry the old `run_after_lte` forward. The details
pane will navigate to the new run, but the Grid can still be constrained to the
previous time window and exclude the run we just created.
Could we preserve only the params that are intended to survive this
transition, such as `limit` and explicit user filters, and either drop or
recompute the Grid window/date params for the newly triggered run?
2. Router Location Source
Could we use `useLocation()` for the current search string instead of
reading `globalThis.location.search` directly?
This hook already operates inside React Router via `useNavigate()` and
`useParams()`, so keeping the location read in the same abstraction makes the
behavior easier to reason about and test. It also avoids coupling this hook to
the browser global, which is awkward in `MemoryRouter` tests and any
non-browser render path.
--
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]
