Skip to content

Commit

Permalink
fix(app): fix post attach pipette calibration double wizard (#7008)
Browse files Browse the repository at this point in the history
Fix general bugginess associated with the change pipette wizard launching a pipette offset
calibration wizard from it's own instance of the useCalibratePipetteOffset hook, while the
pipetteInfo is still rendering in the background with it's own identical instance of the hook. This
prevents both wizards from rendering on top of eachother and is a stop gap fix to get us to a world
where we have a cleaner definition of global app state that's likely not divided between redux and
react-router.
  • Loading branch information
b-cooper authored Nov 16, 2020
1 parent 47fdbb5 commit f4ec258
Show file tree
Hide file tree
Showing 11 changed files with 58 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -161,13 +161,13 @@ export function useCalibratePipetteOffset(
)
)
}
const isCorrectSession =
pipOffsetCalSession &&
mount === pipOffsetCalSession.createParams.mount &&
tipRackDefinition === pipOffsetCalSession.createParams.tipRackDefinition

const Wizard =
startingSession ||
(pipOffsetCalSession &&
mount === pipOffsetCalSession.createParams.mount &&
tipRackDefinition ===
pipOffsetCalSession.createParams.tipRackDefinition) ? (
startingSession || isCorrectSession ? (
<Portal level="top">
{startingSession ? (
<SpinnerModalPage
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ export function AskForCalibrationBlockModal(props: Props): React.Node {
title: props.titleBarTitle,
back: { onClick: props.closePrompt, title: EXIT, children: EXIT },
}}
outerProps={{ padding: '4.5rem 1rem 1rem 1rem' }}
>
<Flex flexDirection={DIRECTION_COLUMN} padding={SPACING_3}>
<Flex width="100%" justifyContent={JUSTIFY_SPACE_BETWEEN}>
Expand Down
10 changes: 9 additions & 1 deletion app/src/components/InstrumentSettings/AttachedPipettesCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ type Props = {|
robotName: string,
makeChangeUrl: (mount: Mount) => string,
makeConfigureUrl: (mount: Mount) => string,
isChangingOrConfiguringPipette: boolean,
|}

// TODO(mc, 2019-12-09): i18n
Expand All @@ -40,7 +41,12 @@ const PIPETTES = 'Pipettes'
const FETCH_PIPETTES_INTERVAL_MS = 5000

export function AttachedPipettesCard(props: Props): React.Node {
const { robotName, makeChangeUrl, makeConfigureUrl } = props
const {
robotName,
makeChangeUrl,
makeConfigureUrl,
isChangingOrConfiguringPipette,
} = props
const dispatch = useDispatch<Dispatch>()

const pipettes = useSelector((state: State) =>
Expand Down Expand Up @@ -70,13 +76,15 @@ export function AttachedPipettesCard(props: Props): React.Node {
pipette={pipettes.left}
changeUrl={makeChangeUrl(LEFT)}
settingsUrl={settings.left ? makeConfigureUrl(LEFT) : null}
isChangingOrConfiguringPipette={isChangingOrConfiguringPipette}
/>
<PipetteInfo
robotName={robotName}
mount={RIGHT}
pipette={pipettes.right}
changeUrl={makeChangeUrl(RIGHT)}
settingsUrl={settings.right ? makeConfigureUrl(RIGHT) : null}
isChangingOrConfiguringPipette={isChangingOrConfiguringPipette}
/>
</Flex>
</Card>
Expand Down
11 changes: 9 additions & 2 deletions app/src/components/InstrumentSettings/PipetteCalibrationInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,17 @@ type Props = {|
serialNumber: string | null,
mount: Mount,
disabledReason: string | null,
isChangingOrConfiguringPipette: boolean,
|}

export function PipetteCalibrationInfo(props: Props): React.Node {
const { robotName, serialNumber, mount, disabledReason } = props
const {
robotName,
serialNumber,
mount,
disabledReason,
isChangingOrConfiguringPipette,
} = props
const [tlcTargetProps, tlcTooltipProps] = useHoverTooltip()
const [pocTargetProps, pocTooltipProps] = useHoverTooltip()
const pipetteOffsetCalibration = useSelector((state: State) =>
Expand Down Expand Up @@ -294,7 +301,7 @@ export function PipetteCalibrationInfo(props: Props): React.Node {
{TIP_NOT_CALIBRATED_BODY}
</Text>
)}
{PipetteOffsetCalibrationWizard}
{!isChangingOrConfiguringPipette && PipetteOffsetCalibrationWizard}
{calBlockModalState !== CAL_BLOCK_MODAL_CLOSED ? (
<Portal level="top">
<AskForCalibrationBlockModal
Expand Down
11 changes: 10 additions & 1 deletion app/src/components/InstrumentSettings/PipetteInfo.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ export type PipetteInfoProps = {|
pipette: AttachedPipette | null,
changeUrl: string,
settingsUrl: string | null,
isChangingOrConfiguringPipette: boolean,
|}

const MOUNT = 'mount'
Expand All @@ -52,7 +53,14 @@ const ATTACH = 'attach'
const NONE = 'none'

export function PipetteInfo(props: PipetteInfoProps): React.Node {
const { robotName, mount, pipette, changeUrl, settingsUrl } = props
const {
robotName,
mount,
pipette,
changeUrl,
settingsUrl,
isChangingOrConfiguringPipette,
} = props
const displayName = pipette ? pipette.modelSpecs.displayName : null
const serialNumber = pipette ? pipette.id : null
const channels = pipette ? pipette.modelSpecs.channels : null
Expand Down Expand Up @@ -146,6 +154,7 @@ export function PipetteInfo(props: PipetteInfoProps): React.Node {
serialNumber={serialNumber}
mount={mount}
disabledReason={disabledReason}
isChangingOrConfiguringPipette={isChangingOrConfiguringPipette}
/>
{disabledReason !== null && (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ describe('PipetteInfo', () => {
pipette={pipette}
changeUrl="change/pipette"
settingsUrl="settings/pipette"
isChangingOrConfiguringPipette={false}
/>
)
}
Expand Down
9 changes: 8 additions & 1 deletion app/src/components/InstrumentSettings/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,10 +12,16 @@ type Props = {|
robotName: string,
makeChangePipetteUrl: (mount: Mount) => string,
makeConfigurePipetteUrl: (mount: Mount) => string,
isChangingOrConfiguringPipette: boolean,
|}

export function InstrumentSettings(props: Props): React.Node {
const { robotName, makeChangePipetteUrl, makeConfigurePipetteUrl } = props
const {
robotName,
makeChangePipetteUrl,
makeConfigurePipetteUrl,
isChangingOrConfiguringPipette,
} = props

return (
<CardContainer>
Expand All @@ -24,6 +30,7 @@ export function InstrumentSettings(props: Props): React.Node {
robotName={robotName}
makeChangeUrl={makeChangePipetteUrl}
makeConfigureUrl={makeConfigurePipetteUrl}
isChangingOrConfiguringPipette={isChangingOrConfiguringPipette}
/>
</CardRow>
<CardRow>
Expand Down
4 changes: 3 additions & 1 deletion app/src/pages/Robots/InstrumentSettings.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,20 +13,22 @@ export type InstrumentSettingsProps = {|
robotDisplayName: string,
url: string,
path: string,
pathname: string,
|}

// used to guarantee mount param in route is left or right
const RE_MOUNT = `(${LEFT}|${RIGHT})`

export function InstrumentSettings(props: InstrumentSettingsProps): React.Node {
const { robotName, robotDisplayName, url, path } = props
const { robotName, robotDisplayName, url, path, pathname } = props
const titleBarProps = { title: robotDisplayName }

return (
<>
<Page titleBarProps={titleBarProps}>
<SettingsContent
robotName={robotName}
isChangingOrConfiguringPipette={pathname !== path}
makeChangePipetteUrl={mnt => `${url}/change-pipette/${mnt}`}
makeConfigurePipetteUrl={mnt => `${url}/configure-pipette/${mnt}`}
/>
Expand Down
4 changes: 3 additions & 1 deletion app/src/pages/Robots/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// connect and configure robots page
import * as React from 'react'
import { useSelector } from 'react-redux'
import { useRouteMatch, Redirect } from 'react-router-dom'
import { useRouteMatch, Redirect, useLocation } from 'react-router-dom'

import {
CONNECTABLE,
Expand All @@ -21,6 +21,7 @@ import { InstrumentSettings } from './InstrumentSettings'
export function Robots(): React.Node {
const { path, url, params } = useRouteMatch()
const instrumentsMatch = useRouteMatch(`${path}/instruments`)
const location = useLocation()
const { name } = params

const appUpdate = useSelector(getShellUpdateState)
Expand Down Expand Up @@ -53,6 +54,7 @@ export function Robots(): React.Node {
robotDisplayName={robot.displayName}
url={instrumentsMatch.url}
path={instrumentsMatch.path}
pathname={location && location.pathname}
/>
) : (
<RobotSettings robot={robot} appUpdate={appUpdate} />
Expand Down
10 changes: 5 additions & 5 deletions components/src/__tests__/__snapshots__/modals.test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -160,11 +160,11 @@ exports[`modals Modal renders correctly with optional heading 1`] = `
`;

exports[`modals ModalPage renders correctly 1`] = `
.c1 {
.c0 {
min-width: 0;
}
.c0.spin {
.c1.spin {
-webkit-animation: GLFYz 0.8s steps(8) infinite;
animation: GLFYz 0.8s steps(8) infinite;
-webkit-transform-origin: center;
Expand All @@ -173,7 +173,7 @@ exports[`modals ModalPage renders correctly 1`] = `
}
<div
className="modal_page"
className="c0 modal_page"
>
<div
className="overlay"
Expand All @@ -189,7 +189,7 @@ exports[`modals ModalPage renders correctly 1`] = `
>
<svg
aria-hidden="true"
className="sc-AxmLO c0 icon"
className="sc-AxmLO c1 icon"
fill="currentColor"
version="1.1"
viewBox="0 0 24 24"
Expand Down Expand Up @@ -219,7 +219,7 @@ exports[`modals ModalPage renders correctly 1`] = `
</h2>
</header>
<div
className="c1 modal_page_contents"
className="c0 modal_page_contents"
>
children
</div>
Expand Down
7 changes: 4 additions & 3 deletions components/src/modals/ModalPage.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ export type ModalPageProps = {|
heading?: React.Node,
children?: React.Node,
innerProps?: React.ElementProps<typeof Box>,
outerProps?: React.ElementProps<typeof Box>,
|}

export function ModalPage(props: ModalPageProps): React.Node {
const { titleBar, heading, innerProps = {} } = props
const { titleBar, heading, innerProps = {}, outerProps = {} } = props

return (
<div className={styles.modal_page}>
<Box className={styles.modal_page} {...outerProps}>
<Overlay />
<TitleBar {...titleBar} className={styles.title_bar} />
<Box
Expand All @@ -33,6 +34,6 @@ export function ModalPage(props: ModalPageProps): React.Node {
{heading && <h3 className={styles.modal_heading}>{heading}</h3>}
{props.children}
</Box>
</div>
</Box>
)
}

0 comments on commit f4ec258

Please sign in to comment.