fix(app): refactor Flex pipette card to reference /instruments #14702
Conversation
smb2268
requested review from
brenthagen,
jerader,
ncdiehl11,
sfoster1 and
b-cooper
and removed request for
a team
| transformOrigin="20% -10%" | ||
| /> | ||
| <Flex | ||
| alignItems={ALIGN_CENTER} |
There was a problem hiding this comment.
This is a separate bug, right? Like it's logically equivalent with the new transformOrigin at the standard viewpoint size, but it reflows better?
There was a problem hiding this comment.
Yeah this code wasn't hit previously because we weren't using InstrumentCard for pipettes - so I pulled the logic from what was actually being hit which lived here https://github.com/Opentrons/opentrons/blame/edge/app/src/organisms/Devices/PipetteCard/index.tsx#L261 and was recently updated by Nick to fix a styling bug
| isEstopNotDisengaged, | ||
| }: FlexPipetteCardProps): JSX.Element { | ||
| const { t, i18n } = useTranslation(['device_details', 'shared']) | ||
| const host = useHost() as HostConfig |
There was a problem hiding this comment.
do you need to type this? isn't host already type HostConfig?
| // this gives the instruments endpoint time to start reporting | ||
| // a good instrument | ||
| React.useEffect(() => { | ||
| if (attachedPipette?.ok === false) { |
There was a problem hiding this comment.
nit
| if (attachedPipette?.ok === false) { | |
| if (!attachedPipette?.ok) { |
There was a problem hiding this comment.
I think this needs to be explicit -- we don't want to enter this if statement if attachedPipette is null or undefined, only if it does exist and ok is false
| import { DropTipWizard } from '../../../DropTipWizard' | ||
|
|
||
| import type { PipetteData } from '@opentrons/api-client' | ||
| import { mockLeftSpecs } from '../../../../redux/pipettes/__fixtures__' |
| pipetteSpecs={pipetteModelSpecs} | ||
| mount={mount} | ||
| transform="scale(0.3)" | ||
| transformOrigin={'20% 52%'} |
There was a problem hiding this comment.
nit:
| transformOrigin={'20% 52%'} | |
| transformOrigin="20% 52%" |
| {attachedPipette?.ok && showAboutPipetteSlideout && ( | ||
| <AboutPipetteSlideout | ||
| pipetteId={attachedPipette.serialNumber} | ||
| pipetteName={pipetteDisplayName ?? attachedPipette.instrumentName} | ||
| firmwareVersion={attachedPipette.firmwareVersion} | ||
| isExpanded={showAboutPipetteSlideout} | ||
| onCloseClick={() => setShowAboutPipetteSlideout(false)} | ||
| /> | ||
| )} |
There was a problem hiding this comment.
nit, to keep the same pattern for easier readability:
| {attachedPipette?.ok && showAboutPipetteSlideout && ( | |
| <AboutPipetteSlideout | |
| pipetteId={attachedPipette.serialNumber} | |
| pipetteName={pipetteDisplayName ?? attachedPipette.instrumentName} | |
| firmwareVersion={attachedPipette.firmwareVersion} | |
| isExpanded={showAboutPipetteSlideout} | |
| onCloseClick={() => setShowAboutPipetteSlideout(false)} | |
| /> | |
| )} | |
| {attachedPipette?.ok && showAboutPipetteSlideout ? ( | |
| <AboutPipetteSlideout | |
| pipetteId={attachedPipette.serialNumber} | |
| pipetteName={pipetteDisplayName ?? attachedPipette.instrumentName} | |
| firmwareVersion={attachedPipette.firmwareVersion} | |
| isExpanded={showAboutPipetteSlideout} | |
| onCloseClick={() => setShowAboutPipetteSlideout(false)} | |
| /> | |
| ) : null} |
smb2268
force-pushed
the
app_instrument-card-refactor
branch
from
8454264 to
5292830
Compare
fix RQA-2433
Overview
Currently, the pipette cards on the desktop app for Flex reference
/pipetteswhile those on ODD reference/instruments. This has caused confusion because occasionally the pipettes would be visible on desktop but not ODD. Seth made a change in #14608 that makes the/instrumentsresponse report cached data like/pipettesdoes, which should have eliminated this discrepancy. This PR ensures that by using/instrumentsas the one source of truth for pipettes on Flex.Test Plan
Smoke test pipette cards and all possible overflow menu actions for both Flex and OT-2 pipettes. This includes OT-2 pipette settings, attach, detach, and calibrate flows, drop tip wizard, and the about pipette menu.
Changelog
FlexPipetteCardand pull allFlexspecific logic out ofPipetteCardinto here.PipetteCardandPipetteOverflowMenu, removing allFlexlogic which has been moved toFlexPipetteCardand some leftover OT-2 calibration logic that is no longer used since we only allow pipette cal from the calibration dashboard now.InstrumentsAndModulesonly enableusePipettesQueryfor OT-2 anduseInstrumentsQueryfor Flex. DisplayPipetteCardorFlexPipetteCardaccordingly.Review requests
Look through the code - did I miss anything? And test this out if you can!
Risk assessment
Medium