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

Toggle all params and all metrics on ManageColumnsPopover #105

Conversation

jescalada
Copy link
Collaborator

@jescalada jescalada commented Jul 1, 2024

PR for G-Research/fasttrackml#76.

Note that there was an issue with the Hide All button which wasn't working correctly with lots (5000+) of metrics, and also there were some pretty terrible performance issue when pressing any of the triggers.

This PR aims to fix the Hide All logic, as well as patching some of the performance issues (reduces wait times from around 60 seconds down to 2-3 seconds when pressing any trigger, at the cost of not being able to view/search columns correctly when there are 100+ columns). Users are expected to use the Toggle buttons or automatically hiding unselected metrics/params to begin with.

Changelog

  • Fix Hide All logic (wasn't filtering custom metrics)
  • Add virtualization for middle column to fix performance issues
    • Only active when there are >100 columns in the list
    • BUG: Virtualization doesn't let you view all the columns
    • BUG: (CSS issue) Dragging items makes the item visually disappear (it shifts from view becoming invisible, but the logic works as intended)
  • Add toggles for Metrics and Params
    • Show/Hide All Params
    • Show/Hide All Metrics

@jescalada jescalada self-assigned this Jul 1, 2024
@jescalada jescalada changed the title Toggle column params and metrics Toggle all params and all metrics on ManageColumnsPopover Jul 1, 2024
@jescalada jescalada marked this pull request as ready for review July 15, 2024 17:58
Copy link

@suprjinx suprjinx left a comment

Choose a reason for hiding this comment

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

Looks good

@jescalada jescalada merged commit 56bdd87 into G-Research:release/v3.17.5 Jul 17, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants