Skip to content

Conversation

@tirkarthi
Copy link
Contributor

Thanks @bbovenzi for the new UI page and contribution docs. Looks really nice with built in dark mode from ground up. New build system using Vite and HMR is really nice. I don't need to reload for changes and enables a faster feedback loop with state intact. I tried implementing basic pagination for the new UI page. I opened this PR for initial feedback and some questions on trying out the implementation in new UI. I guess it serves as a good point to discuss and improve upon.

Questions

  1. In the previous API the case of the keys of the response used to be changed using interceptor. In this case will this be applicable to new API/UI? like total_entries will become totalEntries?
  2. Default dags per page is configurable in airflow.cfg . Earlier the values used to be passed along meta tag for rendering. In this case page title and page size are some of the attributes that might be needed. I don't see any endpoint in airflow/www/views.py so I was wondering how to get those values here like the other react implementation. Currently, I just used 10 hard coded as a constant.
  3. The new UI generates API stubs from openapi I guess similar to the other react project. Will this be merged eventually as I see useQuery for other parts with somewhat different conventions compared to the current one where useDagServiceGetDags is used.

Pagination Ref :

https://tanstack.com/table/latest/docs/guide/pagination#manual-server-side-pagination
https://medium.com/@aylo.srd/server-side-pagination-and-sorting-with-tanstack-table-and-react-bd493170125e

Screenshot : (first page, previous page, next page, last page)

image

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

cc: @bbovenzi @pierrejeambrun

@tirkarthi tirkarthi marked this pull request as draft August 30, 2024 18:49
@bbovenzi
Copy link
Contributor

Looking like a good start!

  1. I haven't decided yet if we want to transform the API responses to camelCased. It is definitely more appriopriate to javascript. It may depend on how easy it is to integrate into our code generation flow
  2. We should use the new ui_api to pass config values through the user object or a /config endpoint.
  3. We will use the autogenerated queries like you see used here. We might want to play with the codegen settings or with the openapu.yaml file to improve the naming of everything

@tirkarthi tirkarthi changed the title Implement pagination for the new home page. [WIP] Implement pagination for the new home page. Aug 31, 2024
@tirkarthi
Copy link
Contributor Author

We should use the new ui_api to pass config values through the user object or a /config endpoint.

I made a simple grep in the repo and couldn't find something related to ui_api . Can you please point me to the code so that I can use the same?

@tirkarthi
Copy link
Contributor Author

I have added pagination, page number buttons along with making pagination a separate component since I feel it will be useful in other table oriented pages in future like dagruns, task instances etc.

Demo :

vlc-record-2024-08-31-11h14m35s-simplescreenrecorder-2024-08-31_11.08.40.mp4-.mp4

@bbovenzi
Copy link
Contributor

bbovenzi commented Sep 4, 2024

We should use the new ui_api to pass config values through the user object or a /config endpoint.

I made a simple grep in the repo and couldn't find something related to ui_api . Can you please point me to the code so that I can use the same?

We haven't integrated it yet. That can happen later, I'm just saying that's how we should eventually do it.

I think this PR is a good start! Let's change our hard-coded limit from 10 to 25 or 50 and I think you could mark this as ready for review. We're going to refactor a lot over time anyway.

@tirkarthi tirkarthi marked this pull request as ready for review September 4, 2024 14:15
@tirkarthi
Copy link
Contributor Author

Thanks @bbovenzi , I changed it to 50 and it looks like default value is 100. I noticed the UI PRs from @pierrejeambrun setting up UI API in #41798. I will follow the development along and make changes once the changes are in.

@tirkarthi tirkarthi changed the title [WIP] Implement pagination for the new home page. Implement pagination for the new home page. Sep 4, 2024
@tirkarthi
Copy link
Contributor Author

Fixed PR comments and rebased with latest changes 6708afe

@bbovenzi bbovenzi merged commit 7650f09 into apache:main Sep 4, 2024
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.

2 participants