Skip to content

Commit eb8cf4c

Browse files
brenthagenb-cooper
andauthored
fix(app): fix jump to current step when outside current window (#11052)
run log jump to current step was not working when the viewport was outside of the current command window. this reuses the component logic on mount to jump to the current command window. closes #10898 Co-authored-by: Brian Cooper <[email protected]>
1 parent 09fbb1f commit eb8cf4c

File tree

2 files changed

+69
-60
lines changed

2 files changed

+69
-60
lines changed

app/src/organisms/Devices/ProtocolRun/RunLog.tsx

Lines changed: 63 additions & 58 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ import {
2929
RUN_STATUS_STOPPED,
3030
RUN_STATUS_SUCCEEDED,
3131
} from '@opentrons/api-client'
32-
import { useAllCommandsQuery } from '@opentrons/react-api-client'
32+
import { useAllCommandsQuery, useRunQuery } from '@opentrons/react-api-client'
3333

3434
import { NAV_BAR_WIDTH } from '../../../App/constants'
3535
import { PrimaryButton } from '../../../atoms/buttons'
@@ -48,12 +48,13 @@ import { StepItem } from './StepItem'
4848
import type { RunCommandSummary } from '@opentrons/api-client'
4949
import type { RunTimeCommand, CommandStatus } from '@opentrons/shared-data'
5050

51-
const AVERAGE_ITEM_HEIGHT_PX = 52 // average px height of a command item
51+
const AVERAGE_ITEM_HEIGHT_PX = 56 // average px height of a command item
5252
const WINDOW_OVERLAP = 40 // number of command items that fall within two adjacent windows
5353
const NUM_EAGER_ITEMS = 5 // number of command items away from the end of the current window that will trigger a window transition if scrolled into view
5454
const COMMANDS_REFETCH_INTERVAL = 3000
5555
const AVERAGE_WINDOW_HEIGHT_PX =
5656
(RUN_LOG_WINDOW_SIZE - WINDOW_OVERLAP) * AVERAGE_ITEM_HEIGHT_PX
57+
const NON_COMMAND_BUFFER_PX = 118 // px height of a the area below and above the windowed command items, but within the scrolled div
5758
interface CommandRuntimeInfo {
5859
analysisCommand: RunTimeCommand | null // analysisCommand will only be null if protocol is nondeterministic
5960
runCommandSummary: RunCommandSummary | null
@@ -90,6 +91,8 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
9091
firstPostInitialPlayRunCommandIndex.current != null
9192
? firstPostInitialPlayRunCommandIndex.current
9293
: 0
94+
const isCurrentRun =
95+
useRunQuery(runId, { staleTime: Infinity }).data?.data.current ?? false
9396
const { data: commandsData } = useAllCommandsQuery(
9497
runId,
9598
{
@@ -108,10 +111,9 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
108111
commandsData?.links?.current?.meta?.createdAt ?? null
109112
const runTotalCommandCount = commandsData?.meta?.totalLength
110113

111-
const [
112-
isInitiallyJumpingToCurrent,
113-
setIsInitiallyJumpingToCurrent,
114-
] = React.useState<boolean>(false)
114+
const [isJumpingToCurrent, setIsJumpingToCurrent] = React.useState<boolean>(
115+
false
116+
)
115117

116118
const analysisCommandsWithStatus =
117119
protocolData?.commands != null
@@ -181,7 +183,7 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
181183
windowFirstCommandIndex,
182184
windowFirstCommandIndex + RUN_LOG_WINDOW_SIZE
183185
)
184-
const isFirstWindow = windowIndex === 0
186+
185187
const isFinalWindow =
186188
currentCommandList.length <= windowFirstCommandIndex + RUN_LOG_WINDOW_SIZE
187189

@@ -248,18 +250,23 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
248250
(RUN_LOG_WINDOW_SIZE - WINDOW_OVERLAP)
249251
)
250252

251-
// when we initially mount, if the current item is not in view, jump to it
252-
React.useEffect(() => {
253+
const jumpToCurrentCommandWindow = (): void => {
253254
if (
254255
indexOfFirstWindowContainingCurrentCommand !== windowIndex &&
255256
indexOfLastWindowContainingCurrentCommand !== windowIndex
256257
) {
257258
const targetWindow = isCurrentCommandInFinalWindow
258259
? indexOfFirstWindowContainingCurrentCommand
259260
: indexOfLastWindowContainingCurrentCommand
261+
260262
setWindowIndex(Math.max(targetWindow, 0))
261263
}
262-
setIsInitiallyJumpingToCurrent(true)
264+
setIsJumpingToCurrent(true)
265+
}
266+
267+
// when we initially mount, jump to current command window
268+
React.useEffect(() => {
269+
jumpToCurrentCommandWindow()
263270
}, [])
264271

265272
const scrollToCurrentItem = (): void => {
@@ -269,14 +276,14 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
269276
// if jumping to current item and on correct window index, scroll to current item
270277
React.useEffect(() => {
271278
if (
272-
isInitiallyJumpingToCurrent &&
279+
isJumpingToCurrent &&
273280
(windowIndex === indexOfFirstWindowContainingCurrentCommand ||
274281
windowIndex === indexOfLastWindowContainingCurrentCommand)
275282
) {
276283
scrollToCurrentItem()
277-
setIsInitiallyJumpingToCurrent(false)
284+
setIsJumpingToCurrent(false)
278285
}
279-
}, [windowIndex, isInitiallyJumpingToCurrent])
286+
}, [windowIndex, isJumpingToCurrent])
280287

281288
if (protocolData == null || runStatus == null) return null
282289

@@ -327,15 +334,17 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
327334
setShowDownloadRunLogToast(true)
328335
}
329336

330-
const isRunStarted = currentItemRef.current != null
331-
332-
// check if edges of current step are below or above window
333337
const isCurrentStepBelow =
334-
currentItemRef.current?.getBoundingClientRect().top != null &&
335-
currentItemRef.current?.getBoundingClientRect().top > window.innerHeight
338+
(currentItemRef.current?.getBoundingClientRect().top != null &&
339+
listInnerRef.current?.getBoundingClientRect().bottom != null &&
340+
currentItemRef.current?.getBoundingClientRect().top >
341+
listInnerRef.current?.getBoundingClientRect().bottom) ||
342+
windowIndex < indexOfFirstWindowContainingCurrentCommand
336343
const isCurrentStepAbove =
337-
currentItemRef.current?.getBoundingClientRect().bottom != null &&
338-
currentItemRef.current?.getBoundingClientRect().bottom < 0
344+
(currentItemRef.current?.getBoundingClientRect().bottom != null &&
345+
currentItemRef.current?.getBoundingClientRect().bottom <
346+
NON_COMMAND_BUFFER_PX) ||
347+
windowIndex > indexOfLastWindowContainingCurrentCommand
339348

340349
const jumpToCurrentStepButton = (
341350
<PrimaryButton
@@ -345,11 +354,11 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
345354
transform="translate(-50%)"
346355
borderRadius={SPACING.spacing6}
347356
display={
348-
isRunStarted && (isCurrentStepBelow || isCurrentStepAbove)
357+
isCurrentRun && (isCurrentStepBelow || isCurrentStepAbove)
349358
? DISPLAY_FLEX
350359
: DISPLAY_NONE
351360
}
352-
onClick={scrollToCurrentItem}
361+
onClick={jumpToCurrentCommandWindow}
353362
id="RunLog_jumpToCurrentStep"
354363
>
355364
<Icon
@@ -377,43 +386,39 @@ export function RunLog({ robotName, runId }: RunLogProps): JSX.Element | null {
377386
/>
378387
) : null}
379388
{jumpToCurrentStepButton}
380-
{isFirstWindow ? (
381-
<>
382-
<Flex
383-
justifyContent={JUSTIFY_SPACE_BETWEEN}
384-
alignItems={ALIGN_CENTER}
385-
borderBottom={BORDERS.lineBorder}
386-
padding={SPACING.spacing4}
389+
<Flex
390+
justifyContent={JUSTIFY_SPACE_BETWEEN}
391+
alignItems={ALIGN_CENTER}
392+
borderBottom={BORDERS.lineBorder}
393+
padding={SPACING.spacing4}
394+
>
395+
<Flex alignItems={ALIGN_CENTER}>
396+
<StyledText
397+
marginRight={SPACING.spacing3}
398+
css={TYPOGRAPHY.h3SemiBold}
399+
textTransform={TEXT_TRANSFORM_CAPITALIZE}
387400
>
388-
<Flex alignItems={ALIGN_CENTER}>
389-
<StyledText
390-
marginRight={SPACING.spacing3}
391-
css={TYPOGRAPHY.h3SemiBold}
392-
textTransform={TEXT_TRANSFORM_CAPITALIZE}
393-
>
394-
{t('run_log')}
395-
</StyledText>
396-
<StyledText
397-
as="label"
398-
color={COLORS.darkGreyEnabled}
399-
id="RunLog_totalStepCount"
400-
>
401-
{isDeterministic
402-
? t('total_step_count', { count: currentCommandList.length })
403-
: t('unable_to_determine_steps')}
404-
</StyledText>
405-
</Flex>
406-
<Btn
407-
color={COLORS.darkGreyEnabled}
408-
css={TYPOGRAPHY.labelSemiBold}
409-
onClick={onClickDownloadRunLog}
410-
id="RunLog_downloadRunLog"
411-
>
412-
{t('download_run_log')}
413-
</Btn>
414-
</Flex>
415-
</>
416-
) : null}
401+
{t('run_log')}
402+
</StyledText>
403+
<StyledText
404+
as="label"
405+
color={COLORS.darkGreyEnabled}
406+
id="RunLog_totalStepCount"
407+
>
408+
{isDeterministic
409+
? t('total_step_count', { count: currentCommandList.length })
410+
: t('unable_to_determine_steps')}
411+
</StyledText>
412+
</Flex>
413+
<Btn
414+
color={COLORS.darkGreyEnabled}
415+
css={TYPOGRAPHY.labelSemiBold}
416+
onClick={onClickDownloadRunLog}
417+
id="RunLog_downloadRunLog"
418+
>
419+
{t('download_run_log')}
420+
</Btn>
421+
</Flex>
417422
<Flex
418423
flexDirection={DIRECTION_COLUMN}
419424
padding={SPACING.spacing4}

app/src/organisms/Devices/ProtocolRun/__tests__/RunLog.test.tsx

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { UseQueryResult } from 'react-query'
44
import { fireEvent } from '@testing-library/dom'
55

66
import { renderWithProviders } from '@opentrons/components'
7-
import { useAllCommandsQuery } from '@opentrons/react-api-client'
7+
import { useAllCommandsQuery, useRunQuery } from '@opentrons/react-api-client'
88

99
import { i18n } from '../../../../i18n'
1010
import fixtureAnalysis from '../../../../organisms/RunDetails/__fixtures__/analysis.json'
@@ -20,7 +20,7 @@ import { RunLogProtocolSetupInfo } from '../RunLogProtocolSetupInfo'
2020
import { StepItemComponent as StepItem } from '../StepItem'
2121
import { RunLog } from '../RunLog'
2222

23-
import type { CommandsData } from '@opentrons/api-client'
23+
import type { CommandsData, Run } from '@opentrons/api-client'
2424
import type { ProtocolAnalysisFile } from '@opentrons/shared-data'
2525
import type { RunTimestamps } from '../../../../organisms/RunTimeControl/hooks'
2626

@@ -38,6 +38,7 @@ const mockUseProtocolDetailsForRun = useProtocolDetailsForRun as jest.MockedFunc
3838
const mockUseAllCommandsQuery = useAllCommandsQuery as jest.MockedFunction<
3939
typeof useAllCommandsQuery
4040
>
41+
const mockUseRunQuery = useRunQuery as jest.MockedFunction<typeof useRunQuery>
4142
const mockUseRunStatus = useRunStatus as jest.MockedFunction<
4243
typeof useRunStatus
4344
>
@@ -73,6 +74,9 @@ describe('RunLog', () => {
7374
displayName: 'mock display name',
7475
protocolKey: 'fakeProtocolKey',
7576
})
77+
when(mockUseRunQuery).mockReturnValue(({
78+
data: { data: { current: true } },
79+
} as unknown) as UseQueryResult<Run>)
7680
when(mockUseAllCommandsQuery).mockReturnValue(({
7781
data: { data: runRecord.data.commands, meta: { totalLength: 14 } },
7882
} as unknown) as UseQueryResult<CommandsData>)

0 commit comments

Comments
 (0)