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(protocol-designer): remove flickering in timeline and fix distrib… #16901

Open
wants to merge 10 commits into
base: edge
Choose a base branch
from
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
"duplicate": "Duplicate step",
"edit_step": "Edit step",
"engage_height": "Engage height",
"final_deck_state": "Final deck state",
"final_deck_state": "Ending deck",
jerader marked this conversation as resolved.
Show resolved Hide resolved
"flow_type_title": "{{type}} flow rate",
"from": "from",
"heater_shaker": {
Expand Down Expand Up @@ -104,7 +104,7 @@
"shake": "Shake",
"single": "Single path",
"speed": "Speed",
"starting_deck_state": "Starting deck state",
"starting_deck_state": "Starting deck",
jerader marked this conversation as resolved.
Show resolved Hide resolved
"step_substeps": "{{stepType}} details",
"temperature": "Temperature",
"temperature_module": {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
{
"__end__": "Ending deck",
"__initial_setup__": "Starting deck",
"__presaved_step__": "Unsaved step",
"adapter_compatible_lab": "Adapter compatible labware",
"adapter": "Adapters",
"add_fixture": "Add a fixture",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ interface StepSummaryProps {
export function StepSummary(props: StepSummaryProps): JSX.Element | null {
const { currentStep, stepDetails } = props
const { t } = useTranslation(['protocol_steps', 'application'])

const labwareNicknamesById = useSelector(getLabwareNicknamesById)
const additionalEquipmentEntities = useSelector(
getAdditionalEquipmentEntities
Expand All @@ -91,7 +90,6 @@ export function StepSummary(props: StepSummaryProps): JSX.Element | null {
return null
}
const { stepType } = currentStep

let stepSummaryContent: JSX.Element | null = null
switch (stepType) {
case 'mix':
Expand Down Expand Up @@ -405,7 +403,6 @@ export function StepSummary(props: StepSummaryProps): JSX.Element | null {
default:
stepSummaryContent = null
}

return stepSummaryContent != null || stepDetails != null ? (
<Flex
flexDirection={DIRECTION_COLUMN}
Expand All @@ -414,7 +411,9 @@ export function StepSummary(props: StepSummaryProps): JSX.Element | null {
>
{stepSummaryContent != null ? (
<ListItem type="noActive">
<Flex padding={SPACING.spacing12}>{stepSummaryContent}</Flex>
<Flex padding={SPACING.spacing12} height="4.75rem">
{stepSummaryContent}
</Flex>
</ListItem>
) : null}
{stepDetails != null && stepDetails !== '' ? (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -132,24 +132,25 @@ function SubstepComponent(props: SubstepProps): JSX.Element {
</ListItem>
) : (
<>
<ListItem type="noActive">
<Flex
gridGap={SPACING.spacing4}
padding={SPACING.spacing12}
justifyContent={JUSTIFY_SPACE_BETWEEN}
width="100%"
alignItems={ALIGN_CENTER}
>
{ingredIds.length > 0 ? (
<Flex gridGap={SPACING.spacing4} alignItems={ALIGN_CENTER}>
<LiquidIcon color={color} size="medium" />
{source != null ? (
<ListItem type="noActive">
<Flex
gridGap={SPACING.spacing4}
padding={SPACING.spacing12}
justifyContent={JUSTIFY_SPACE_BETWEEN}
width="100%"
alignItems={ALIGN_CENTER}
>
{ingredIds.length > 0 ? (
<Flex gridGap={SPACING.spacing4} alignItems={ALIGN_CENTER}>
<LiquidIcon color={color} size="medium" />

<StyledText desktopStyle="bodyDefaultRegular">
{ingredIds.map(groupId => ingredNames[groupId]).join(',')}
</StyledText>
</Flex>
) : null}

<StyledText desktopStyle="bodyDefaultRegular">
{ingredIds.map(groupId => ingredNames[groupId]).join(',')}
</StyledText>
</Flex>
) : null}
{source != null ? (
<Flex gridGap={SPACING.spacing4} alignItems={ALIGN_CENTER}>
<StyledText desktopStyle="bodyDefaultRegular">
{t('protocol_steps:aspirated')}
Expand All @@ -164,11 +165,11 @@ function SubstepComponent(props: SubstepProps): JSX.Element {
})}
/>
</Flex>
) : null}
</Flex>
</ListItem>
<ListItem type="noActive">
{dest !== undefined ? (
</Flex>
</ListItem>
) : null}
{dest != null ? (
Copy link
Collaborator

Choose a reason for hiding this comment

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

should we also remove the dest != null check on line 188 then?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think that is needed for when you consolidate, right? because you'll have only one dispense to many aspirates.

<ListItem type="noActive">
<Flex
gridGap={SPACING.spacing4}
padding={SPACING.spacing12}
Expand Down Expand Up @@ -206,8 +207,8 @@ function SubstepComponent(props: SubstepProps): JSX.Element {
</Flex>
) : null}
</Flex>
) : null}
</ListItem>
</ListItem>
) : null}
</>
)}
</Flex>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
import {
getSelectedStepId,
getSelectedSubstep,
getSelectedTerminalItemId,
} from '../../../../ui/steps/selectors'
import { getDesignerTab } from '../../../../file-data/selectors'
import { getEnableHotKeysDisplay } from '../../../../feature-flags/selectors'
Expand Down Expand Up @@ -60,6 +61,7 @@ describe('ProtocolSteps', () => {
vi.mocked(DeckSetupContainer).mockReturnValue(
<div>mock DeckSetupContainer</div>
)
vi.mocked(getSelectedTerminalItemId).mockReturnValue(null)
vi.mocked(OffDeck).mockReturnValue(<div>mock OffDeck</div>)
vi.mocked(getUnsavedForm).mockReturnValue(null)
vi.mocked(getSelectedSubstep).mockReturnValue(null)
Expand Down
18 changes: 10 additions & 8 deletions protocol-designer/src/pages/Designer/ProtocolSteps/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
DIRECTION_COLUMN,
Flex,
JUSTIFY_CENTER,
JUSTIFY_FLEX_END,
JUSTIFY_FLEX_START,
JUSTIFY_SPACE_BETWEEN,
POSITION_FIXED,
Expand All @@ -27,6 +26,7 @@ import {
getSelectedSubstep,
getSelectedStepId,
getHoveredStepId,
getSelectedTerminalItemId,
} from '../../../ui/steps/selectors'
import { DeckSetupContainer } from '../DeckSetup'
import { OffDeck } from '../Offdeck'
Expand All @@ -42,6 +42,7 @@ const CONTENT_MAX_WIDTH = '46.9375rem'
export function ProtocolSteps(): JSX.Element {
const { i18n, t } = useTranslation('starting_deck_state')
const formData = useSelector(getUnsavedForm)
const selectedTerminalItem = useSelector(getSelectedTerminalItemId)
const isMultiSelectMode = useSelector(getIsMultiSelectMode)
const selectedSubstep = useSelector(getSelectedSubstep)
const enableHoyKeyDisplay = useSelector(getEnableHotKeysDisplay)
Expand Down Expand Up @@ -89,14 +90,12 @@ export function ProtocolSteps(): JSX.Element {
{tab === 'protocolSteps' ? (
<TimelineAlerts justifyContent={JUSTIFY_CENTER} width="100%" />
) : null}
<Flex
justifyContent={
currentStep != null ? JUSTIFY_SPACE_BETWEEN : JUSTIFY_FLEX_END
}
>
{currentStep != null ? (
<Flex justifyContent={JUSTIFY_SPACE_BETWEEN}>
{currentStep != null || selectedTerminalItem != null ? (
<StyledText desktopStyle="headingSmallBold">
{i18n.format(currentStep.stepName, 'capitalize')}
{currentStep != null
? i18n.format(currentStep.stepName, 'capitalize')
: t(selectedTerminalItem)}
</StyledText>
) : null}
<ToggleGroup
Expand All @@ -123,6 +122,9 @@ export function ProtocolSteps(): JSX.Element {
stepDetails={stepDetails}
/>
) : null}
{selectedTerminalItem != null && currentHoveredStepId == null ? (
<Flex height="4.75rem" width="100%" />
) : null}
</Flex>
</Flex>
{enableHoyKeyDisplay ? (
Expand Down
Loading