-
Notifications
You must be signed in to change notification settings - Fork 180
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(app): add module calibrate button to protocol setup #13380
feat(app): add module calibrate button to protocol setup #13380
Conversation
add calibrate button to Desktop app protocol setup close RAUT-555
Codecov Report
@@ Coverage Diff @@
## chore_release-7.0.0 #13380 +/- ##
=======================================================
- Coverage 71.69% 71.58% -0.12%
=======================================================
Files 2427 2427
Lines 67572 67764 +192
Branches 7782 7858 +76
=======================================================
+ Hits 48443 48506 +63
- Misses 17301 17420 +119
- Partials 1828 1838 +10
Flags with carried forward coverage won't be shown. Click here to find out more.
|
moduleFromProtocol={heaterShakerModuleFromProtocol} | ||
attachedModule={heaterShakerAttachedModule} | ||
<> | ||
{showModuleWizard && attachedModuleMatch != null ? ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{showModuleWizard && attachedModuleMatch != null ? ( | |
{showModuleWizard ? ( |
Nitpicking, but Is the attachedModuleMatch check necessary here? The only way we can show the wizard is if the button is clicked?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added attachedModuleMatch != null
because of ModuleWiardFlowProps
.
interface ModuleWizardFlowsProps {
attachedModule: AttachedModule
closeFlow: () => void
slotName?: string
onComplete?: () => void
}
Actually I wrote {showModuleWizard ? (
first and got an error.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tested and works. Lgtm with the one question 🚀
* feat(app): add calibrate button to Desktop app protocol setup
Overview
Add module calibrate button to protocol setup and update tests
close RAUT-555
Test Plan
Changelog
Review requests
Risk assessment
low