Skip to content

I15-1: Get container info on playlist and display if it's not mounted - #159

Open
DominicOram wants to merge 6 commits into
mainfrom
115_gather_container_location_in_playlist
Open

I15-1: Get container info on playlist and display if it's not mounted#159
DominicOram wants to merge 6 commits into
mainfrom
115_gather_container_location_in_playlist

Conversation

@DominicOram

Copy link
Copy Markdown
Contributor

Part of #115

To test:

  • Confirm the fakeExperiments structure in handlers matches what is returned in gql from the new getSession... query
  • Run in mock mode and see that the row that is not mounted shows a warning and has an appropriate tooltip

@DominicOram
DominicOram marked this pull request as ready for review August 18, 2026 12:47
@DominicOram

Copy link
Copy Markdown
Contributor Author

With #163 merged i'll fix the typing on this now

@jacob-williamson jacob-williamson left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, just one question.

Also a small UX nit, I think the warning message may be more discoverable if the row had a warning icon somewhere, probably on the far right. And then if you hovered over this warning icon, it could tell you the message, similar to how we want to show the errors when we hover over the error icon in the queue view. Happy to merge as is though and we can add this in later if we want to

enableDensityToggle: false,
enableFullScreenToggle: false,
muiTableBodyRowProps: ({ row }) => {
const showWarning = hasContainerNotOnTable(row.original);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Shouldn't we also flag if a sample's container is null? As this presumably means we don't know where it is

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.

2 participants