Skip to content

LIMS-1683: Show buttons for next/previous container on plate view#1069

Open
ndg63276 wants to merge 2 commits into
masterfrom
improvement/LIMS-1683/add-next-prev-buttons-to-plates
Open

LIMS-1683: Show buttons for next/previous container on plate view#1069
ndg63276 wants to merge 2 commits into
masterfrom
improvement/LIMS-1683/add-next-prev-buttons-to-plates

Conversation

@ndg63276
Copy link
Copy Markdown
Collaborator

@ndg63276 ndg63276 commented May 8, 2026

JIRA ticket: LIMS-1683

Summary:

#912 added buttons to move to the next/previous container in the puck view, this PR is to add the same buttons to the plate views.

Changes:

  • Move scoped CSS for the header row and the flat buttons into the general use area
  • Add prev/next buttons to the view both of plates that have been imaged and those that haven't
  • Add code to get sibling containers, work out the current container number, and which container ids are next/previous, to add titles to the buttons, and disable when necessary
  • Always do the container sorting in the backend for consistency, and use container id to distinguish containers with the same name
  • Remove unused function updateContainerSampleGroupsData and related functions fetchDewars and fetchContainers and related variable containersCollection.

To test:

  • Go to a puck with siblings (eg /containers/cid/367188), check the Next and Prev Container buttons still appear, look the same as on prod, have titles when you hover, and disable correctly when on the first / last puck of the dewar.
  • Go to a plate that has been imaged (eg /containers/cid/373688), check the Next and Prev Container buttons appear and behave the same as for pucks
  • Go to (or create) a plate that has not been imaged (eg /containers/cid/373685), check the Next and Prev Container buttons appear and behave the same as for pucks
  • Add a puck and a plate to a dewar with an already imaged plate, check you can move between the 3 kinds as expected

await result.fetch();
}

result = new Containers(null, { state: { pageSize: 9999 } });
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.

Nitpick, but could result be initialised like

let result = new Containers(null, { state: { pageSize: 9999 } });?

Comment thread client/src/css/partials/_utility.scss Outdated
&.danger {
@apply tw-bg-red-500 tw-text-white;
&:hover, &:focus {
@apply tw-bg-red-700
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.

Suggested change
@apply tw-bg-red-700
@apply tw-bg-red-700;

let foundCurrent = false;
this.siblingContainers.each((container, index) => {
let idx = index+1
if (foundCurrent && !this.nextContainer) {
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.

Should this come after foundCurrent is set? (line 779)

As it currently is, it might just not reflect the actual condition because it is evaluated before the containerId check is executed

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

No, this is deliberate. You are looping through all the containers (in order), so you need to have already found the currently-displayed one, (and not yet set the 'next' one), in order to set this container as the 'next' one.

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.

Yeah, that makes sense, I'll approve it now, thanks for having a look!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants