Skip to content

Conversation

@tirkarthi
Copy link
Contributor

@tirkarthi tirkarthi commented Dec 24, 2024

Closes #33210

There are workflows where users need to quickly switch between dags. Returning to home page to search takes time and also loads dags list data and makes other queries which are not needed. Add a search widget in the top navbar so that users can search and results are returned like legacy home page search. Then users can click on the result to switch to the relevant dag page. The component uses AsyncSelect to fetch options on type and uses debounce to limit queries. The results are limited to 10 so that the queries are faster and uses existing dags endpoint used by home page search without recent runs data.

Notes to reviewer and self

  • debounce-promise is needed since use-debounce doesn't work with promises . Ref : Async Debounce for Select 2.0 not behaving as expected JedWatson/react-select#3075 (comment)
  • staleTime is set as zero so that queries are not cached and results are live to search for dags added recently after a search for the term.
  • There are autocomplete components for Chakra but they are not compatible with Chakra 3 . Ref : Support for chakra v3 anubra266/choc-autocomplete#272
  • There could be a custom endpoint with just dag_id and dag_display_name but with 10 entries the size is low at 2kB compressed and 8-9kB uncompressed and doesn't seem to be worth it to have a separate endpoint. Probably if needed can be taken as an enhancement in another PR.

dags_search_modal

@boring-cyborg boring-cyborg bot added the area:UI Related to UI/UX. For Frontend Developers. label Dec 24, 2024
Copy link
Contributor

@shubhamraj-git shubhamraj-git left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks good.

@tirkarthi
Copy link
Contributor Author

@shubhamraj-git The search is substring search like the legacy view. Legacy view has limit of 10 entries and I am fine with increasing the limit here. Thanks.

for row in session.execute(dag_ids_query.union(owners_query).order_by("name").limit(10))

@shubhamraj-git
Copy link
Contributor

@shubhamraj-git The search is substring search like the legacy view. Legacy view has limit of 10 entries and I am fine with increasing the limit here. Thanks.

for row in session.execute(dag_ids_query.union(owners_query).order_by("name").limit(10))

Actually yes, I saw in the legacy. Hence removed the comment, since i got my answer. Yes, maybe increasing to 10 will be good.

@tirkarthi
Copy link
Contributor Author

Thanks, increased it to 10 and also increased width to view longer names.

@jscheffl
Copy link
Contributor

jscheffl commented Jan 1, 2025

Note: As PR #45312 has been merged, the code formatting rules have changed for new UI. Please rebase and re-run pre-commit checks to ensure that formatting in folder airflow/ui is adjusted.

@bbovenzi
Copy link
Contributor

bbovenzi commented Jan 3, 2025

I think we should style this as a universal search like how the Chakra 3 docs work:

Screenshot 2025-01-03 at 10 04 54 AM

@tirkarthi
Copy link
Contributor Author

@bbovenzi I don't find any docs in Chakra for search. Is it just making the dropdown component display in modal and make the modal open on clicking on search?

@Kysluss
Copy link

Kysluss commented Jan 5, 2025

Hello,

choc-autocomplete just put out an alpha release that adds v3 support. Feel free to use it and submit any feedback, issues, and PRs. I'm one of the maintainers of the project and we're trying to get it stable as quick as possible.

Edit Official v6 is out and stable that adds support for Chakra v3

@bbovenzi
Copy link
Contributor

bbovenzi commented Jan 7, 2025

@bbovenzi I don't find any docs in Chakra for search. Is it just making the dropdown component display in modal and make the modal open on clicking on search?

Yeah, its not a specific component. but built using the first Searchbar as a button and then opening a dropdown in a modal. Although we'll need it to be a typeahead search and not a set dropdown.

@tirkarthi
Copy link
Contributor Author

Thanks @bbovenzi , updated the PR so that "search dags" at top right of the header is a button and it opens the modal with the search implementation. Updated screenshot in the PR description.

@tirkarthi
Copy link
Contributor Author

Chakra search is also triggered by "ctrl+k" . I guess there needs to be a tracking ticket to track porting keyboard shortcuts in legacy UI to new UI too.

@tirkarthi
Copy link
Contributor Author

Hello,

choc-autocomplete just put out an alpha release that adds v3 support. Feel free to use it and submit any feedback, issues, and PRs. I'm one of the maintainers of the project and we're trying to get it stable as quick as possible.

Edit Official v6 is out and stable that adds support for Chakra v3

Thanks @Kysluss , will check it out.

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. Let's add a similar keyboard command. Maybe we maintain a single json file of keyboard commands that components need to import so we can maintain a single source of truth.

  2. We should have a default list of dags ready with the dropdown already open when the user opens the dialog. That can either be just whatever 10 dags the API returns or later we can have it be the recently viewed dags.

@tirkarthi
Copy link
Contributor Author

  1. Let's add a similar keyboard command. Maybe we maintain a single json file of keyboard commands that components need to import so we can maintain a single source of truth.

I have not worked on keyboard shortcuts in the legacy UI and need to understand this. From the initial understanding it seems the keypress is captured and an event is dispatched which gets handled by a reducer. I would like to have it as a separate issue since it looks like lot of work that would need it's own ticket.

2. We should have a default list of dags ready with the dropdown already open when the user opens the dialog. That can either be just whatever 10 dags the API returns or later we can have it be the recently viewed dags.

Thanks @bbovenzi for the suggestion. react-select library provides an option to just set defaulOptions with list of options or to pass defaultOptions that calls the API. In this case I took the later approach where dag_display_pattern will be empty and the first 10 dags are returned.

https://react-select.com/async#defaultoptions

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can make the keyboard controls another PR

But one last comment, Let's make a src/components/SearchDags directory and put our three components inside since we only actually use SearchDagsButton.

@tirkarthi
Copy link
Contributor Author

Thanks, refactored to SearchDags folder exporting only SearchDagsButton in index.ts . I have also set menuIsOpen so that default dags are always shown on modal open and not just on clicking the select input.

Copy link
Contributor

@bbovenzi bbovenzi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

great work!

@bbovenzi bbovenzi merged commit 39d7f1c into apache:main Jan 8, 2025
35 checks passed
HariGS-DB pushed a commit to HariGS-DB/airflow that referenced this pull request Jan 16, 2025
* Add search dags widget to navbar.

* Use null instead of undefined to clear selected value.

* Remove unused async.

* Increase limit to 10 and width to accommodate longer names.

* Rebase and run format for line number rule.

* Move search to a modal like Chakra docs.

* Set default list of dags on initial render.

* Refactor components to SearchDags folder.
got686-yandex pushed a commit to got686-yandex/airflow that referenced this pull request Jan 30, 2025
* Add search dags widget to navbar.

* Use null instead of undefined to clear selected value.

* Remove unused async.

* Increase limit to 10 and width to accommodate longer names.

* Rebase and run format for line number rule.

* Move search to a modal like Chakra docs.

* Set default list of dags on initial render.

* Refactor components to SearchDags folder.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:UI Related to UI/UX. For Frontend Developers.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Enhanced DAG Navigation with Search Bar on Dag View Page

5 participants