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

fix(protocol-designer): remove flickering in timeline and fix distrib… #16901

Open
wants to merge 10 commits into
base: edge
Choose a base branch
from

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Nov 20, 2024

…ute details

closes RQA-3630 RQA-3624

Overview

This PR does 2 things:

  1. fixes the step details when you have distributed something
  2. adds fixed box to step summary to avoid flickering, updates copy for terminal steps, and adds a title copy for terminal text

Test Plan and Hands on Testing

Create a protocol and make a distribute step: basically aspirate from 1 well and dispense into multiple wells and add a small volume like 10uL and the distribute path should be selectable. Then click "view details" on the step's overflow menu and see that it looks as expected with no extra blank list items

Then upload the attached protocol and hover through the steps in the timeline, see that there is no flickering

1.0 testing.json

Changelog

  • only show aspirate list item if there is an aspirate well present
  • add a fixed box to avoid flickering in protocol steps
  • add copy to terminal step items

Risk assessment

low

@jerader jerader requested a review from a team as a code owner November 20, 2024 13:36
@jerader jerader requested a review from koji November 20, 2024 17:28
</Flex>
</ListItem>
) : null}
{dest != null ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we also remove the dest != null check on line 188 then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think that is needed for when you consolidate, right? because you'll have only one dispense to many aspirates.

Comment on lines +123 to +124
"block": "<text>End with block at</text><tag/>",
"lid_position": "<text>and lid</text><tag/>"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

recommended by Ed to shorten this

@ncdiehl11
Copy link
Collaborator

ncdiehl11 commented Nov 21, 2024

Screenshot 2024-11-21 at 4 07 49 PM

I think we need to format uppercase the DeckInfoLabel deckLabel translation, and omit liquid icon?

Copy link
Collaborator

@ncdiehl11 ncdiehl11 left a comment

Choose a reason for hiding this comment

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

🚀 thank you for cleaning up and fixing the overflow menu items as well!

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.

4 participants