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]
