Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(coral) - View environments: Add tables rendering list of environments #1761

Merged
merged 12 commits into from
Sep 15, 2023

Conversation

mathieu-anderson
Copy link
Contributor

@mathieu-anderson mathieu-anderson commented Sep 13, 2023

About this change - What it does

Add table and search field for each environment tab

Screen.Recording.2023-09-13.at.15.28.01.mov

Notes

  • There is a double call to getPaginatedEnvironmentsForTopicAndAcl on first load of the page, because we call it in EnvironmentsTabs to get the amount of env as badges. However, the call in EnvironmentsTabs has refetchOnMount: false, which will prevent another call until a full page reload. I am not sure how to avoid this.
  • The search field takes the full width of the table, which is a little disgraceful. Maybe a subsequent change to TableLayout could address this issue (setting a max width to the filters?)

Follow up

The Status values do not currently have the timestamp value, as this PR #1747 has not been merged yet. I started working on a follow up PR.

Resolves: #1707

Mathieu Anderson added 4 commits September 13, 2023 10:42
Signed-off-by: Mathieu Anderson <[email protected]>
Signed-off-by: Mathieu Anderson <[email protected]>
Signed-off-by: Mathieu Anderson <[email protected]>
Signed-off-by: Mathieu Anderson <[email protected]>
@mathieu-anderson mathieu-anderson self-assigned this Sep 13, 2023
@mathieu-anderson mathieu-anderson added enhancement New feature or request Frontend Relates to coral (react app) labels Sep 13, 2023
@mathieu-anderson mathieu-anderson changed the title 1707 add env tables feat(coral) - View environments: Add tables rendering list of environments Sep 13, 2023
@mathieu-anderson mathieu-anderson marked this pull request as ready for review September 13, 2023 13:35
@programmiri
Copy link
Contributor

There is a double call to getPaginatedEnvironmentsForTopicAndAcl on first load of the page, because we call it in EnvironmentsTabs to get the amount of env as badges. However, the call in EnvironmentsTabs has refetchOnMount: false, which will prevent another call until a full page reload. I am not sure how to avoid this.

Ah, that's tricky 🤔

the query in e.g. KafkaEnvironment (["getPaginatedEnvironmentsForTopicAndAcl", currentPage, search]) is a different query as in EnvironmentTabs (["getPaginatedEnvironmentsForTopicAndAcl", 1]) which is why when KafkaEnvironment it will not use the getPaginatedEnvironmentsForTopicAndAcl from the cache.

I... I think this could now be the case where we should think about using a custom hook for this? So it's always the same query (with currentPage and search etc) and we can make use of the cache properly.

@mathieu-anderson
Copy link
Contributor Author

I think a query hook would be great for this use case. I'm going to try it out ^^

@mathieu-anderson
Copy link
Contributor Author

@programmiri I pushed the changes we talked about:

  • refreshing environment status
  • abstracting data fetching in a query hook so that there will not be a double call when fetching the badge numbers and the table data
Screen.Recording.2023-09-14.at.12.09.46.mov

Copy link
Contributor

@programmiri programmiri left a comment

Choose a reason for hiding this comment

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

🎉 for the custom hook, solved the problem nicely!

@programmiri programmiri merged commit d7ec44f into main Sep 15, 2023
@programmiri programmiri deleted the 1707-add-env-tables branch September 15, 2023 07:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Frontend Relates to coral (react app)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(coral) - View environments: Add tables rendering list of environments
2 participants