Skip to content

I15-1: Show playlists from selected session - #150

Open
DominicOram wants to merge 4 commits into
mainfrom
149_use_selected_session_for_playlist
Open

I15-1: Show playlists from selected session#150
DominicOram wants to merge 4 commits into
mainfrom
149_use_selected_session_for_playlist

Conversation

@DominicOram

Copy link
Copy Markdown
Contributor

Fixes #149

To test:

  • Run the mocked UI
  • Switch the session with the selector in the top bar
  • Confirm that the playlist changes

@EmsArnold EmsArnold left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

One change which would be good to make, and some slight comments which can be overlooked.

};
});

const mockedUseSubmitTask = queueService.useSumbitQueueTask as unknown as Mock;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nit: mocked variables should either be mockUse... or mockedUse..., to be consistent. I believe there is some special case for mock... in Vitest, though obviously mocked... likely matches the pattern that Vitest looks for.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I agree. It looks like we currently have a mix throughout the codebase but mostly look consistent within a file. Fixing this across the board feels out of scope for this PR though, I have made #155

);
};

export function parseInstrumentSession(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should: Rather than adding an additional function, you could use visitTextToVisit from app-shell/src/utils, which parses the visit text into a visit, which you can then pull the proposal number and session number out of. This may need to be exported.

If this is something which needs to be done more regularly, we should likely look at adding this into sci-react-ui as well.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I've updated to use visitTextToVisit. I think #156 might be a nicer solution but seems out of scope

@DominicOram
DominicOram requested a review from EmsArnold August 14, 2026 11:56
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.

I15-1: Use the instrument session select in the playlist view

2 participants