Skip to content

Commit 3884f09

Browse files
jeraderjerader
authored
fix(protocol-visualization, app): no longer glitching when scrolling (#21686)
closes RQA-5498
1 parent a9438a2 commit 3884f09

10 files changed

Lines changed: 268 additions & 128 deletions

File tree

‎app/src/organisms/Desktop/ProtocolDetails/AnnotatedSteps/AnnotatedGroup.tsx‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
import { useLayoutEffect, useState } from 'react'
2+
import clsx from 'clsx'
23

34
import { COLORS, Icon, StepGroup } from '@opentrons/components'
45

56
import styles from './annotatedsteps.module.css'
67
import { IndividualCommand } from './IndividualCommand'
8+
import {
9+
getExpandedGroupBodyMaxHeightPx,
10+
shouldCapExpandedGroupBodyHeight,
11+
} from './utils'
712

813
import type { Dispatch, ReactNode, SetStateAction } from 'react'
914
import type {
@@ -13,11 +18,6 @@ import type {
1318
} from '@opentrons/shared-data'
1419
import type { LeafNode } from '/app/redux/protocol-storage'
1520

16-
/** Reserve space for StepGroup header (title row, optional subtitle, padding). */
17-
const STEP_GROUP_HEADER_RESERVE_PX = 80
18-
const MIN_EXPANDED_BODY_PX = 160
19-
const LIST_VIEWPORT_BOTTOM_BUFFER_PX = 8
20-
2121
interface AnnotatedGroupProps {
2222
scrollTargetId: string | null
2323
listElement: HTMLElement | null
@@ -70,15 +70,14 @@ export function AnnotatedGroup(props: AnnotatedGroupProps): JSX.Element {
7070
command => command.isHighlighted
7171
)
7272

73-
const expandedMaxHeightPx =
74-
listViewportHeight > 0
75-
? Math.max(
76-
MIN_EXPANDED_BODY_PX,
77-
listViewportHeight -
78-
STEP_GROUP_HEADER_RESERVE_PX -
79-
LIST_VIEWPORT_BOTTOM_BUFFER_PX
80-
)
81-
: null
73+
const shouldCapExpandedBody = shouldCapExpandedGroupBodyHeight({
74+
subCommandCount: subCommands.length,
75+
listViewportHeight,
76+
hasTrailingErrors,
77+
})
78+
const expandedMaxHeightPx = shouldCapExpandedBody
79+
? getExpandedGroupBodyMaxHeightPx(listViewportHeight)
80+
: null
8281

8382
return (
8483
<div className={styles.annotated_group_container}>
@@ -102,12 +101,15 @@ export function AnnotatedGroup(props: AnnotatedGroupProps): JSX.Element {
102101
>
103102
{isExpanded ? (
104103
<div
105-
className={styles.annotated_group_expanded}
106-
style={
107-
expandedMaxHeightPx != null
108-
? { maxHeight: expandedMaxHeightPx }
109-
: {}
110-
}
104+
className={clsx(styles.annotated_group_expanded, {
105+
[styles.annotated_group_expanded_natural_height]:
106+
!shouldCapExpandedBody,
107+
})}
108+
style={{
109+
...(expandedMaxHeightPx != null && {
110+
maxHeight: expandedMaxHeightPx,
111+
}),
112+
}}
111113
>
112114
{subCommands.map((subCommand, index) => (
113115
<IndividualCommand

‎app/src/organisms/Desktop/ProtocolDetails/AnnotatedSteps/IndividualCommand.tsx‎

Lines changed: 49 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { useEffect, useRef } from 'react'
1+
import { useLayoutEffect, useRef } from 'react'
22
import clsx from 'clsx'
33

44
import { COLORS, CommandText } from '@opentrons/components'
@@ -27,6 +27,28 @@ interface IndividualCommandProps {
2727
commandNumber: number
2828
setSelectedCommand?: Dispatch<SetStateAction<string | null>>
2929
}
30+
31+
function scrollContainerToShowTarget(
32+
container: HTMLElement,
33+
target: HTMLElement
34+
): boolean {
35+
const containerRect = container.getBoundingClientRect()
36+
const targetRect = target.getBoundingClientRect()
37+
const isBelow = targetRect.bottom >= containerRect.bottom - 8
38+
const isAbove = targetRect.top <= containerRect.top + 1
39+
40+
if (!isBelow && !isAbove) {
41+
return false
42+
}
43+
44+
const nextTop = container.scrollTop + (targetRect.top - containerRect.top)
45+
container.scrollTo({
46+
behavior: 'auto',
47+
top: Math.max(0, nextTop),
48+
})
49+
return true
50+
}
51+
3052
export function IndividualCommand({
3153
command,
3254
analysis,
@@ -41,50 +63,36 @@ export function IndividualCommand({
4163
const commandRef = useRef<HTMLDivElement | null>(null)
4264
const iconColor = isHighlighted ? COLORS.purple50 : COLORS.grey50
4365

44-
useEffect(() => {
66+
useLayoutEffect(() => {
4567
if (!isHighlighted || commandRef.current == null) return
4668
if (command.id !== scrollTargetId) return
4769

48-
requestAnimationFrame(() => {
49-
const commandEl = commandRef.current
50-
if (commandEl == null) return
51-
52-
const groupExpandedEl =
53-
fromGroup === true
54-
? commandEl.closest<HTMLElement>(
55-
`.${styles.annotated_group_expanded}`
56-
)
57-
: null
58-
59-
const container: HTMLElement | null =
60-
groupExpandedEl instanceof HTMLElement
61-
? groupExpandedEl
62-
: (listElement ??
63-
commandEl.closest<HTMLElement>('[role="list"]') ??
64-
null)
65-
66-
if (container == null) {
67-
commandEl.scrollIntoView({
68-
behavior: 'smooth',
69-
block: 'nearest',
70-
inline: 'nearest',
71-
})
72-
return
73-
}
74-
75-
const containerRect = container.getBoundingClientRect()
76-
const commandRect = commandEl.getBoundingClientRect()
77-
const isBelow = commandRect.bottom >= containerRect.bottom - 8
78-
const isAbove = commandRect.top <= containerRect.top + 1
79-
if (!isBelow && !isAbove) return
80-
81-
const nextTop =
82-
container.scrollTop + (commandRect.top - containerRect.top)
83-
container.scrollTo({
84-
behavior: 'smooth',
85-
top: Math.max(0, nextTop),
70+
const commandEl = commandRef.current
71+
const groupExpandedEl =
72+
fromGroup === true
73+
? commandEl.closest<HTMLElement>(`.${styles.annotated_group_expanded}`)
74+
: null
75+
const outerListEl =
76+
listElement ?? commandEl.closest<HTMLElement>('[role="list"]') ?? null
77+
78+
let scrollContainer: HTMLElement | null = outerListEl
79+
80+
if (groupExpandedEl instanceof HTMLElement) {
81+
const hasInnerScroll =
82+
groupExpandedEl.scrollHeight > groupExpandedEl.clientHeight + 1
83+
scrollContainer = hasInnerScroll ? groupExpandedEl : outerListEl
84+
}
85+
86+
if (scrollContainer == null) {
87+
commandEl.scrollIntoView({
88+
behavior: 'auto',
89+
block: 'nearest',
90+
inline: 'nearest',
8691
})
87-
})
92+
return
93+
}
94+
95+
scrollContainerToShowTarget(scrollContainer, commandEl)
8896
}, [isHighlighted, scrollTargetId, command.id, listElement, fromGroup])
8997

9098
const commandWrapStyle = clsx(styles.individual_command_wrap, {

‎app/src/organisms/Desktop/ProtocolDetails/AnnotatedSteps/annotatedsteps.module.css‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,11 @@
4040
overflow-y: auto;
4141
}
4242

43+
.annotated_group_expanded_natural_height {
44+
max-height: none;
45+
overflow-y: visible;
46+
}
47+
4348
.annotated_group_expanded::-webkit-scrollbar {
4449
display: none;
4550
}

‎app/src/organisms/Desktop/ProtocolDetails/AnnotatedSteps/index.tsx‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -319,7 +319,7 @@ export function AnnotatedSteps(props: AnnotatedStepsProps): JSX.Element {
319319
const [listViewportHeight, setListViewportHeight] = useState(0)
320320
const dynamicRowHeight = useDynamicRowHeight({
321321
defaultRowHeight: DEFAULT_ROW_HEIGHT_PX,
322-
key: `${listWidth}-${listViewportHeight}-${rows.length}`,
322+
key: `${listWidth}-${rows.length}`,
323323
})
324324

325325
useEffect(() => {
@@ -342,8 +342,11 @@ export function AnnotatedSteps(props: AnnotatedStepsProps): JSX.Element {
342342
if (scrollTargetId == null) return
343343
const rowIndex = rowIndexByCommandId.get(scrollTargetId)
344344
if (rowIndex == null) return
345-
listRef?.scrollToRow({ index: rowIndex, align: 'auto' })
346-
}, [scrollTargetId, rowIndexByCommandId, listRef])
345+
listRef?.scrollToRow({
346+
index: rowIndex,
347+
align: rowIndex >= rows.length - 1 ? 'end' : 'auto',
348+
})
349+
}, [scrollTargetId, rowIndexByCommandId, listRef, rows.length])
347350

348351
useEffect(() => {
349352
setListElement(listRef?.element ?? null)

‎app/src/organisms/Desktop/ProtocolDetails/AnnotatedSteps/utils.ts‎

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,57 @@
11
import type { RunTimeCommand } from '@opentrons/shared-data'
22

3+
// space for StepGroup header (title row, optional subtitle, padding)
4+
export const STEP_GROUP_HEADER_RESERVE_PX = 80
5+
export const MIN_EXPANDED_BODY_PX = 160
6+
export const LIST_VIEWPORT_BOTTOM_BUFFER_PX = 8
7+
// matches AnnotatedSteps DEFAULT_ROW_HEIGHT_PX.
8+
export const ESTIMATED_COMMAND_HEIGHT_PX = 64
9+
export const TRAILING_ERRORS_FOOTER_ESTIMATE_PX = 100
10+
11+
export function getExpandedGroupBodyMaxHeightPx(
12+
listViewportHeight: number
13+
): number | null {
14+
if (listViewportHeight <= 0) {
15+
return null
16+
}
17+
18+
return Math.max(
19+
MIN_EXPANDED_BODY_PX,
20+
listViewportHeight -
21+
STEP_GROUP_HEADER_RESERVE_PX -
22+
LIST_VIEWPORT_BOTTOM_BUFFER_PX
23+
)
24+
}
25+
26+
export function getEstimatedExpandedGroupContentHeightPx(params: {
27+
subCommandCount: number
28+
hasTrailingErrors?: boolean
29+
}): number {
30+
const { subCommandCount, hasTrailingErrors = false } = params
31+
32+
return (
33+
subCommandCount * ESTIMATED_COMMAND_HEIGHT_PX +
34+
(hasTrailingErrors ? TRAILING_ERRORS_FOOTER_ESTIMATE_PX : 0)
35+
)
36+
}
37+
38+
// only cap expanded stepGroup height when content would exceed the list viewport
39+
export function shouldCapExpandedGroupBodyHeight(params: {
40+
subCommandCount: number
41+
listViewportHeight: number
42+
hasTrailingErrors?: boolean
43+
}): boolean {
44+
const bodyMaxHeightPx = getExpandedGroupBodyMaxHeightPx(
45+
params.listViewportHeight
46+
)
47+
48+
if (bodyMaxHeightPx == null) {
49+
return false
50+
}
51+
52+
return getEstimatedExpandedGroupContentHeightPx(params) > bodyMaxHeightPx
53+
}
54+
355
export const getIsVisibleProtocolStep = (command: RunTimeCommand): boolean => {
456
return !command.commandType.includes('load') && command.commandType !== 'home'
557
}

‎protocol-visualization/src/organisms/AnnotatedSteps/AnnotatedGroup.tsx‎

Lines changed: 22 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,14 @@
11
import { useLayoutEffect, useState } from 'react'
2+
import clsx from 'clsx'
23

34
import { COLORS, Icon, StepGroup } from '@opentrons/components'
45

56
import styles from './annotatedsteps.module.css'
67
import { IndividualCommand } from './IndividualCommand'
8+
import {
9+
getExpandedGroupBodyMaxHeightPx,
10+
shouldCapExpandedGroupBodyHeight,
11+
} from './utils'
712

813
import type { Dispatch, ReactNode, SetStateAction } from 'react'
914
import type {
@@ -13,11 +18,6 @@ import type {
1318
} from '@opentrons/shared-data'
1419
import type { LeafNode } from '../../types'
1520

16-
/** Reserve space for StepGroup header (title row, optional subtitle, padding). */
17-
const STEP_GROUP_HEADER_RESERVE_PX = 80
18-
const MIN_EXPANDED_BODY_PX = 160
19-
const LIST_VIEWPORT_BOTTOM_BUFFER_PX = 8
20-
2121
interface AnnotatedGroupProps {
2222
scrollTargetId: string | null
2323
listElement: HTMLElement | null
@@ -70,15 +70,14 @@ export function AnnotatedGroup(props: AnnotatedGroupProps): JSX.Element {
7070
command => command.isHighlighted
7171
)
7272

73-
const expandedMaxHeightPx =
74-
listViewportHeight > 0
75-
? Math.max(
76-
MIN_EXPANDED_BODY_PX,
77-
listViewportHeight -
78-
STEP_GROUP_HEADER_RESERVE_PX -
79-
LIST_VIEWPORT_BOTTOM_BUFFER_PX
80-
)
81-
: null
73+
const shouldCapExpandedBody = shouldCapExpandedGroupBodyHeight({
74+
subCommandCount: subCommands.length,
75+
listViewportHeight,
76+
hasTrailingErrors,
77+
})
78+
const expandedMaxHeightPx = shouldCapExpandedBody
79+
? getExpandedGroupBodyMaxHeightPx(listViewportHeight)
80+
: null
8281

8382
return (
8483
<div className={styles.annotated_group_container}>
@@ -102,12 +101,15 @@ export function AnnotatedGroup(props: AnnotatedGroupProps): JSX.Element {
102101
>
103102
{isExpanded ? (
104103
<div
105-
className={styles.annotated_group_expanded}
106-
style={
107-
expandedMaxHeightPx != null
108-
? { maxHeight: expandedMaxHeightPx }
109-
: {}
110-
}
104+
className={clsx(styles.annotated_group_expanded, {
105+
[styles.annotated_group_expanded_natural_height]:
106+
!shouldCapExpandedBody,
107+
})}
108+
style={{
109+
...(expandedMaxHeightPx != null && {
110+
maxHeight: expandedMaxHeightPx,
111+
}),
112+
}}
111113
>
112114
{subCommands.map((subCommand, index) => (
113115
<IndividualCommand

0 commit comments

Comments
 (0)