-
Notifications
You must be signed in to change notification settings - Fork 1.7k
HParam: Implement new Data Table projection architecture in Runs Table #6438
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
HParam: Implement new Data Table projection architecture in Runs Table #6438
Conversation
bd75305 to
9ee7d2b
Compare
9ee7d2b to
fd5fe2a
Compare
| } | ||
| .row-circle > span { | ||
| border-radius: 50%; | ||
| border: 1px solid rgba(255, 255, 255, 0.4); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Please use a color from the color sheet instead of an arbitrary rgba value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how it is done in many places. I am leaving it for now. Maybe I will come do a broad change for all circle icons.
| await TestBed.configureTestingModule({ | ||
| imports: [DataTableModule, MatIconTestingModule], | ||
| declarations: [TestableComponent, RunsDataTable], | ||
| // schemas: [NO_ERRORS_SCHEMA], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // schemas: [NO_ERRORS_SCHEMA], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! Done.
## Motivation for features / changes In #6438 we rewrote the Runs Data Table to use the new projection Data Table structure(Created in #6427 and #6422). This allows us to project custom controls such as the run selection check boxes. This PR implements those checkboxes. ## Technical description of changes Pretty straight forward use of projection. The weird part was that the projected checkbox in the header did not fire the "change" event while the header itself had a subscription to a "click" event for sorting. The solution was to subscribe to the "click" event and that seems to work just fine. ## Screenshots of UI changes (or N/A) All selected: <img width="230" alt="Screenshot 2023-06-20 at 2 40 19 PM" src="https://github.com/tensorflow/tensorboard/assets/8672809/164ab4c2-b2bd-47ad-b46d-9dbcf78c976a"> Some selected: <img width="228" alt="Screenshot 2023-06-20 at 2 40 27 PM" src="https://github.com/tensorflow/tensorboard/assets/8672809/0822e363-ac08-45a3-9102-4c1959192c9f"> None selected: <img width="211" alt="Screenshot 2023-06-20 at 2 40 35 PM" src="https://github.com/tensorflow/tensorboard/assets/8672809/9ce0b8aa-182d-4796-a59b-5e05fc655f2e">
Motivation for features / changes
We preparation for adding HParam functionality to both the Scalar Table and the Runs Table we have refactored the Data Table to be more flexible. This refactoring was done in #6422 and #6427 and was only implemented in the Scalar Table because the Runs Table only uses the Data Table behind a flag. This PR implements the new Data Table Structure in the Runs Table.
Technical description of changes
Basically just followed the same structure as the Scalar Table.
Screenshots of UI changes (or N/A)
Runs Table specific styling still needs to be done. It currently looks like this.

We still need to add a column for check boxes and add styling.