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

feat(protocol-designer, components): add dropdown field deck highlights #17122

Merged
merged 18 commits into from
Dec 18, 2024

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Dec 16, 2024

closes AUTH-1124

Overview

Screen.Recording.2024-12-17.at.09.19.12.mov

This PR adds deck highlights when hovering and selecting the dropdown options for several fields:

  • all module selection fields
  • aspirate and dispense labware
  • mix labware
  • move labware and new location

Additionally, this PR cleans how we auto-populate the dropdown field if only one is available.

Test Plan and Hands on Testing

There is a lot to test here, basically test that the highlight occurs for each module step, mix, transfer and move labware. After you create the steps, reopen them and see that the highlight is auto-populated back in

Changelog

  • add some new redux items for hovering and selecting the dropdown item
  • add new highlight layers for labware, modules, empty slot and offdeck
  • add some test coverage
  • fix how we auto-populate dropdown fields with only one item

Risk assessment

low

@jerader jerader changed the title feat(protocol-designer): add dropdown field deck highlights feat(protocol-designer, components): add dropdown field deck highlights Dec 17, 2024
@@ -118,6 +120,64 @@ const _patchDefaultDropTipLocation = (args: {
return null
}

const _patchDefaultLabwareLocations = (args: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this patch auto-populates labware locations for aspirate_labware, dispense_labware, and labware (in mix) when there is 1 item available. Before, we were using useEffects in the form toolbox and this way is more clean and follows the pattern of other auto-populated fields

@jerader jerader marked this pull request as ready for review December 17, 2024 14:14
@jerader jerader requested review from a team as code owners December 17, 2024 14:14
@jerader jerader requested review from mjhuff, koji and ncdiehl11 and removed request for a team December 17, 2024 14:14
@jerader
Copy link
Collaborator Author

jerader commented Dec 17, 2024

showed this to Felix and there are a few action items remaining

  • highlight ot-2 fixed trash
  • change copy for selected in a transfer to source and destination

@ncdiehl11
Copy link
Collaborator

Mostly looks cool! I notice that the deck label svgs are probably not being mapped in the right order producing occlusion:
Screenshot 2024-12-17 at 10 09 58 AM

Also, some weirdness is going on with the off deck labware rendering
Screenshot 2024-12-17 at 10 12 18 AM

Also looks like some selected labware highlights are persisting even when selecting another step. Here, the mix occurs in the thermocycler plate, but the HS is still highlighted from the previous step selection.
Screenshot 2024-12-17 at 10 13 29 AM

@jerader
Copy link
Collaborator Author

jerader commented Dec 17, 2024

Mostly looks cool! I notice that the deck label svgs are probably not being mapped in the right order producing occlusion:
Also, some weirdness is going on with the off deck labware rendering
Also looks like some selected labware highlights are persisting even when selecting another step. Here, the mix occurs in the thermocycler plate, but the HS is still highlighted from the previous step selection.

@ncdiehl11 thanks! good catch on all of these, will take a look

@jerader jerader removed request for a team and mjhuff December 17, 2024 15:28
@jerader jerader requested a review from koji December 18, 2024 15:49
@@ -59,7 +61,7 @@ export function DeckLabel({

return (
<Flex
fontSize="6px"
fontSize={isZoomed ? '6px' : '18px'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does this come from 3x zooming somewhere? If so, maybe we should have a zoom scalar constant that can be reused

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

it comes from the designs. DeckLabel was altered before because when you zoom into the viewbox, everything rendered in it is also zoomed in, including the text. 18px is the original text size for when you're not zoomed in

@jerader jerader force-pushed the pd_highlight-labware-protocol-steps branch from 6676596 to b1ed0b9 Compare December 18, 2024 17:50
Copy link

codecov bot commented Dec 18, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.00%. Comparing base (906d841) to head (b1ed0b9).
Report is 50 commits behind head on edge.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##             edge   #17122       +/-   ##
===========================================
- Coverage   92.43%   74.00%   -18.44%     
===========================================
  Files          77       43       -34     
  Lines        1283     3250     +1967     
===========================================
+ Hits         1186     2405     +1219     
- Misses         97      845      +748     
Flag Coverage Δ
g-code-testing ?
shared-data 74.00% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 120 files with indirect coverage changes

@jerader jerader requested review from ncdiehl11 and koji December 18, 2024 18:34
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.

Tested on sandbox and works really well. It's a big PR so it's possible we missed something, but I tested every step successfully with a variety of on and off-deck labware, nested on modules/adapters, etc., and the UI functions great. I will keep an eye out for any possible bugs during dev work pre-8.3 release, but I'm comfortable merging. Awesome work

@jerader jerader merged commit a2e4bdd into edge Dec 18, 2024
45 checks passed
@jerader jerader deleted the pd_highlight-labware-protocol-steps branch December 18, 2024 19:00
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.

3 participants