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

Add "Load More" button for Metrics table in Runs Overview tab #110

Conversation

jescalada
Copy link
Collaborator

@jescalada jescalada commented Jul 26, 2024

PR for G-Research/fasttrackml#1355.

Note: This was implemented with a "Load More" button instead of infinite scrolling due to how complicated the default BaseTable is (the metrics list is implemented using this). It was a lot easier to extend the Card to allow loading more rows on demand.

image

Changelog

  • Extended useRunMetricsBatch to allow fetching metrics in groups (default 500) with a startIndex and count
  • Extended Card.tsx to allow conditionally rendering a "Load More" button if there are more than 500 rows
    • loadMore (boolean) and loadMoreHandler must be supplied
    • For this particular use case, loadMore is true (therefore button is rendered) only when the number of loaded metric batches is less than the total number of metrics

@jescalada jescalada self-assigned this Jul 26, 2024
@jescalada jescalada changed the base branch from main to release/v3.17.5 July 26, 2024 16:11
@jescalada jescalada linked an issue Jul 26, 2024 that may be closed by this pull request
@jescalada jescalada marked this pull request as ready for review July 30, 2024 18:12
@jescalada jescalada changed the title Add infinite scrolling for Metrics table in Runs Overview tab Add "Load More" button for Metrics table in Runs Overview tab Jul 30, 2024
@suprjinx
Copy link
Contributor

Implementation looks good! I'm a little confused by the "Load More" button -- maybe place below table? I think it would also be reasonable to just put a note that says "Only showing 500 metrics" and leave it at that.

@jescalada
Copy link
Collaborator Author

Implementation looks good! I'm a little confused by the "Load More" button -- maybe place below table? I think it would also be reasonable to just put a note that says "Only showing 500 metrics" and leave it at that.

@suprjinx

I thought about putting it under the table, however in this case users with 1920x1080 screen would be forced to scroll down a bit to see the button. I wanted to set it in the most logical place, next to the other buttons in the Table, however the RunsOverviewTabMetricsCard component doesn't allow this, and it would require modifying the DataList component which is deep within the hierarchy (and has many other components using it).

Here's another solution I found: We can just display the number of loaded metrics and the total number for clarity. This will only show up when the user has over 500 metrics (and will disappear when all metrics are loaded):

image

@jescalada jescalada merged commit c57eaf6 into G-Research:release/v3.17.5 Aug 1, 2024
4 checks passed
vinayan3 pushed a commit to vinayan3/fasttrackml-ui-aim that referenced this pull request Aug 28, 2024
…arch#110)

* Add index/count based fetching for useRunMetricsBatch

* Add quick fix for alignment config error

* Add preliminary infinite scrolling properties

* Add Load More button to Card

* Fix and clean up useRunMetricsBatch hook

* Type annotations and cleanup

* Clean up old changes

* Fix styling and add dynamic metric count to button

* Fix broken test date check
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integrate infinit scroll
2 participants