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(app): labware setup H-S latch command add runId #11133

Merged
merged 2 commits into from
Jul 15, 2022

Conversation

jerader
Copy link
Collaborator

@jerader jerader commented Jul 14, 2022

closes #11048 & #11125

Overview

The labware setup extra attention banner for the H-S has a button that allows you to open/close the latch. However, the latch command didn't include the runId which is required to send commands to modules during setup.

Changelog

  • refactor command logic to include runId in ModuleExtraAttention and fix test

Review requests

  • does logic sound correct? Try to test this on a h-s if you can! I am still unable to upload H-S protocols for some reason

Risk assessment

low

@jerader jerader requested a review from a team as a code owner July 14, 2022 14:25
@jerader jerader requested review from vabruzzo, sakibh and shlokamin and removed request for a team July 14, 2022 14:25
@codecov
Copy link

codecov bot commented Jul 14, 2022

Codecov Report

Merging #11133 (fa283b8) into edge (f097d56) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             edge   #11133   +/-   ##
=======================================
  Coverage   73.62%   73.63%           
=======================================
  Files        2085     2085           
  Lines       57593    57593           
  Branches     5741     5741           
=======================================
+ Hits        42404    42408    +4     
+ Misses      13951    13949    -2     
+ Partials     1238     1236    -2     
Flag Coverage Δ
app 70.92% <100.00%> (+0.03%) ⬆️

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

Impacted Files Coverage Δ
...src/organisms/Devices/ProtocolRun/SetupLabware.tsx 90.00% <ø> (ø)
...nisms/Devices/ProtocolRun/ModuleExtraAttention.tsx 70.00% <100.00%> (+10.00%) ⬆️
...olSetup/RunSetupCard/ModuleSetup/Banner/Banner.tsx 100.00% <0.00%> (+20.00%) ⬆️

Copy link
Contributor

@sakibh sakibh left a comment

Choose a reason for hiding this comment

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

The code LGTM!

@@ -113,7 +116,8 @@ const ModuleExtraAttentionItem = (
}

const toggleLatch = (): void => {
Copy link
Contributor

Choose a reason for hiding this comment

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

You can also use the useLatchControls hook where it now can take in an optional runId for controls that have a run associated with them. It returns a toggleLatch function as well. Although, this will require passing in the AttachedModule as the first parameter.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yeah, i tried using useLatchControls initially but was having a hard time getting HeaterShakerAttachedModule, so not using it was the cleanest way I think. But I can look into it more!

@jerader jerader merged commit 75cc2f2 into edge Jul 15, 2022
@jerader jerader deleted the app_hs-module-require-extra-attention-banner-latch branch July 15, 2022 16:23
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.

6.0 Feedback: Close labware latch button in Labware setup is unresponsive
2 participants