Skip to content

Commit b5233bc

Browse files
feat(protocol-designer): Update Shift-click step selection to work with concurrent Thermocycler profiles (#20193)
## Overview Protocol Designer lets the user Shift-click to select a range of steps in the timeline. Now that the timeline has nesting (concurrent Thermocycler profiles), we need to update that logic to account for some hazards when the range crosses the boundary of a nested group. This PR does that. Closes EXEC-2060. See that ticket for a more full example of the problem we're solving. ## Changelog * Update the logic in `getShiftSelectedSteps()` to avoid selecting steps that are not "user-visible"—i.e., the hidden "wait for profile to complete" step that implicitly comes at the end of every Thermocycler profile. Some of the other step CRUD actions are already doing the same thing. * Various small renames for clarity. * Delete `SelectMultipleStepsForGroupAction`, which was unused (never created, nor used in any reducer).
1 parent e84b8c2 commit b5233bc

File tree

5 files changed

+155
-64
lines changed

5 files changed

+155
-64
lines changed

protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/ConnectedStepInfo.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ export function ConnectedStepInfo(props: ConnectedStepInfoProps): JSX.Element {
9696
const hoveredStep = useSelector(getHoveredStepId)
9797
const selectedStepId = useSelector(getSelectedStepId)
9898
const multiSelectItemIds = useSelector(getMultiSelectItemIds)
99-
const orderedStepIds = useSelector(stepFormSelectors.getOrderedStepIds)
99+
const stepHierarchy = useSelector(stepFormSelectors.getSavedStepHierarchy)
100100
const lastMultiSelectedStepId = useSelector(getMultiSelectLastSelected)
101101
const isMultiSelectMode = useSelector(getIsMultiSelectMode)
102102
const selected: boolean =
@@ -155,7 +155,7 @@ export function ConnectedStepInfo(props: ConnectedStepInfoProps): JSX.Element {
155155
if (isShiftKeyPressed) {
156156
stepsToSelect = getShiftSelectedSteps(
157157
selectedStepId,
158-
orderedStepIds,
158+
stepHierarchy,
159159
stepId,
160160
multiSelectItemIds,
161161
lastMultiSelectedStepId

protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/__tests__/utils.test.tsx

Lines changed: 82 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,11 @@
11
import { describe, expect, it } from 'vitest'
22

3-
import { capitalizeFirstLetterAfterNumber } from '../utils'
3+
import {
4+
capitalizeFirstLetterAfterNumber,
5+
getShiftSelectedSteps,
6+
} from '../utils'
7+
8+
import type { StepHierarchy } from '/protocol-designer/steplist/utils/stepHierarchy'
49

510
describe('capitalizeFirstLetterAfterNumber', () => {
611
it('should capitalize the first letter of a step type', () => {
@@ -12,3 +17,79 @@ describe('capitalizeFirstLetterAfterNumber', () => {
1217
)
1318
})
1419
})
20+
21+
describe('getShiftSelectedSteps', () => {
22+
it('should return a single step if no steps were selected before', () => {
23+
const stepHierarchy: StepHierarchy = {
24+
topLevelItems: [
25+
{ type: 'standaloneStep', stepId: 'step1' },
26+
{ type: 'standaloneStep', stepId: 'step2' },
27+
{ type: 'standaloneStep', stepId: 'step3' },
28+
],
29+
}
30+
expect(
31+
getShiftSelectedSteps(null, stepHierarchy, 'step2', null, null)
32+
).toStrictEqual(['step2'])
33+
})
34+
it('should return a range if a single step was selected before', () => {
35+
const stepHierarchy: StepHierarchy = {
36+
topLevelItems: [
37+
{ type: 'standaloneStep', stepId: 'step1' },
38+
{
39+
type: 'thermocyclerProfileGroup',
40+
thermocyclerProfileStepId: 'step2',
41+
concurrentSteps: [
42+
{ type: 'standaloneStep', stepId: 'step3' },
43+
{ type: 'standaloneStep', stepId: 'step4' },
44+
],
45+
waitForThermocyclerProfileStepId: 'step5',
46+
},
47+
{ type: 'standaloneStep', stepId: 'step6' },
48+
{ type: 'standaloneStep', stepId: 'step7' },
49+
],
50+
}
51+
expect(
52+
getShiftSelectedSteps('step3', stepHierarchy, 'step7', null, null)
53+
).toStrictEqual([
54+
'step3',
55+
'step4',
56+
// step5 should be skipped because it's hidden in the UI
57+
'step6',
58+
'step7',
59+
])
60+
})
61+
it('should return a range if multiple steps were selected before', () => {
62+
const stepHierarchy: StepHierarchy = {
63+
topLevelItems: [
64+
{ type: 'standaloneStep', stepId: 'step1' },
65+
{
66+
type: 'thermocyclerProfileGroup',
67+
thermocyclerProfileStepId: 'step2',
68+
concurrentSteps: [
69+
{ type: 'standaloneStep', stepId: 'step3' },
70+
{ type: 'standaloneStep', stepId: 'step4' },
71+
],
72+
waitForThermocyclerProfileStepId: 'step5',
73+
},
74+
{ type: 'standaloneStep', stepId: 'step6' },
75+
{ type: 'standaloneStep', stepId: 'step7' },
76+
],
77+
}
78+
expect(
79+
getShiftSelectedSteps(
80+
null,
81+
stepHierarchy,
82+
'step7',
83+
['step2', 'step3'],
84+
'step3'
85+
)
86+
).toStrictEqual([
87+
'step2',
88+
'step3',
89+
'step4',
90+
// step5 should be skipped because it's hidden in the UI
91+
'step6',
92+
'step7',
93+
])
94+
})
95+
})

protocol-designer/src/pages/Designer/ProtocolSteps/Timeline/utils.ts

Lines changed: 53 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,12 @@ import round from 'lodash/round'
22
import uniq from 'lodash/uniq'
33
import { UAParser } from 'ua-parser-js'
44

5+
import { getStepVisibilities } from '/protocol-designer/steplist/utils/getStepVisibilities'
6+
import { convertStepHierarchyToArray } from '/protocol-designer/steplist/utils/stepHierarchy'
7+
58
import type { MouseEvent } from 'react'
69
import type { StepIdType } from '/protocol-designer/form-types'
10+
import type { StepHierarchy } from '/protocol-designer/steplist/utils/stepHierarchy'
711

812
export const capitalizeFirstLetterAfterNumber = (title: string): string =>
913
title.replace(
@@ -36,84 +40,98 @@ export const formatPercentage = (part: number, total: number): string => {
3640
}
3741

3842
export const getMetaSelectedSteps = (
39-
multiSelectItemIds: StepIdType[] | null,
40-
stepId: StepIdType,
41-
selectedStepId: StepIdType | null
43+
priorMultiSelectedItemIds: StepIdType[] | null,
44+
newlySelectedStepId: StepIdType,
45+
priorSingleSelectedStepId: StepIdType | null
4246
): StepIdType[] => {
4347
let stepsToSelect: StepIdType[]
44-
if (multiSelectItemIds?.length) {
48+
if (priorMultiSelectedItemIds?.length) {
4549
// already have a selection, add/remove the meta-clicked item
46-
stepsToSelect = multiSelectItemIds.includes(stepId)
47-
? multiSelectItemIds.filter(id => id !== stepId)
48-
: [...multiSelectItemIds, stepId]
49-
} else if (selectedStepId && selectedStepId === stepId) {
50+
stepsToSelect = priorMultiSelectedItemIds.includes(newlySelectedStepId)
51+
? priorMultiSelectedItemIds.filter(id => id !== newlySelectedStepId)
52+
: [...priorMultiSelectedItemIds, newlySelectedStepId]
53+
} else if (
54+
priorSingleSelectedStepId &&
55+
priorSingleSelectedStepId === newlySelectedStepId
56+
) {
5057
// meta-clicked on the selected single step
51-
stepsToSelect = [selectedStepId]
52-
} else if (selectedStepId) {
58+
stepsToSelect = [priorSingleSelectedStepId]
59+
} else if (priorSingleSelectedStepId) {
5360
// meta-clicked on a different step, multi-select both
54-
stepsToSelect = [selectedStepId, stepId]
61+
stepsToSelect = [priorSingleSelectedStepId, newlySelectedStepId]
5562
} else {
5663
// meta-clicked on a step when a terminal item was selected
57-
stepsToSelect = [stepId]
64+
stepsToSelect = [newlySelectedStepId]
5865
}
5966
return stepsToSelect
6067
}
6168

6269
export const getShiftSelectedSteps = (
63-
selectedStepId: StepIdType | null,
64-
orderedStepIds: StepIdType[],
65-
stepId: StepIdType,
66-
multiSelectItemIds: StepIdType[] | null,
70+
priorSingleSelectedStepId: StepIdType | null,
71+
stepHierarchy: StepHierarchy,
72+
newlySelectedStepId: StepIdType,
73+
priorMultiSelectedItemIds: StepIdType[] | null,
6774
lastMultiSelectedStepId: StepIdType | null
6875
): StepIdType[] => {
6976
let stepsToSelect: StepIdType[]
70-
if (selectedStepId) {
71-
stepsToSelect = getOrderedStepsInRange(
72-
selectedStepId,
73-
stepId,
74-
orderedStepIds
77+
if (priorSingleSelectedStepId) {
78+
stepsToSelect = getOrderedVisibleStepsInRange(
79+
priorSingleSelectedStepId,
80+
newlySelectedStepId,
81+
stepHierarchy
7582
)
76-
} else if (multiSelectItemIds?.length && lastMultiSelectedStepId) {
77-
const potentialStepsToSelect = getOrderedStepsInRange(
83+
} else if (priorMultiSelectedItemIds?.length && lastMultiSelectedStepId) {
84+
const potentialStepsToSelect = getOrderedVisibleStepsInRange(
7885
lastMultiSelectedStepId,
79-
stepId,
80-
orderedStepIds
86+
newlySelectedStepId,
87+
stepHierarchy
8188
)
8289

8390
const allSelected: boolean = potentialStepsToSelect
8491
.slice(1)
85-
.every(stepId => multiSelectItemIds.includes(stepId))
92+
.every(stepId => priorMultiSelectedItemIds.includes(stepId))
8693

8794
if (allSelected) {
8895
// if they're all selected, deselect them all
89-
if (multiSelectItemIds.length - potentialStepsToSelect.length > 0) {
90-
stepsToSelect = multiSelectItemIds.filter(
96+
if (
97+
priorMultiSelectedItemIds.length - potentialStepsToSelect.length >
98+
0
99+
) {
100+
stepsToSelect = priorMultiSelectedItemIds.filter(
91101
(id: StepIdType) => !potentialStepsToSelect.includes(id)
92102
)
93103
} else {
94104
// unless deselecting them all results in none being selected
95105
stepsToSelect = [potentialStepsToSelect[0]]
96106
}
97107
} else {
98-
stepsToSelect = uniq([...multiSelectItemIds, ...potentialStepsToSelect])
108+
stepsToSelect = uniq([
109+
...priorMultiSelectedItemIds,
110+
...potentialStepsToSelect,
111+
])
99112
}
100113
} else {
101-
stepsToSelect = [stepId]
114+
stepsToSelect = [newlySelectedStepId]
102115
}
103116
return stepsToSelect
104117
}
105118

106-
const getOrderedStepsInRange = (
119+
const getOrderedVisibleStepsInRange = (
107120
lastSelectedStepId: StepIdType,
108121
stepId: StepIdType,
109-
orderedStepIds: StepIdType[]
122+
stepHierarchy: StepHierarchy
110123
): StepIdType[] => {
124+
const orderedStepIds = convertStepHierarchyToArray(stepHierarchy)
125+
const stepVisibilities = getStepVisibilities(stepHierarchy)
126+
111127
const prevIndex: number = orderedStepIds.indexOf(lastSelectedStepId)
112128
const currentIndex: number = orderedStepIds.indexOf(stepId)
113-
114129
const [startIndex, endIndex] = [prevIndex, currentIndex].sort((a, b) => a - b)
115-
const orderedSteps = orderedStepIds.slice(startIndex, endIndex + 1)
116-
return orderedSteps
130+
131+
const orderedVisibleSteps = orderedStepIds
132+
.slice(startIndex, endIndex + 1)
133+
.filter(stepId => stepVisibilities[stepId].isVisibleToUser)
134+
return orderedVisibleSteps
117135
}
118136

119137
export const nonePressed = (keysPressed: boolean[]): boolean =>

protocol-designer/src/ui/steps/actions/actions.ts

Lines changed: 18 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,17 +28,18 @@ import type {
2828
selectDropdownItemAction,
2929
Selection,
3030
SelectMultipleStepsAction,
31-
SelectMultipleStepsForGroupAction,
3231
SelectStepAction,
3332
SelectTerminalItemAction,
3433
SetWellSelectionLabwareKeyAction,
3534
ToggleViewSubstepAction,
3635
ViewSubstep,
3736
} from './types'
3837

39-
// adds an incremental integer ID for Step reducers.
40-
// NOTE: if this is an "add step" directly performed by the user,
41-
// addAndSelectStepWithHints is probably what you want
38+
/**
39+
* adds an incremental integer ID for Step reducers.
40+
* NOTE: if this is an "add step" directly performed by the user,
41+
* addAndSelectStepWithHints is probably what you want
42+
*/
4243
export const addStep = (args: {
4344
stepType: StepType
4445
robotStateTimeline: Timeline
@@ -54,10 +55,12 @@ export const addStep = (args: {
5455
},
5556
}
5657
}
58+
5759
export const hoverSelection = (args: Selection): hoverSelectionAction => ({
5860
type: 'HOVER_DROPDOWN_ITEM',
5961
payload: { id: args.id, text: args.text },
6062
})
63+
6164
export const selectDropdownItem = (args: {
6265
selection: Selection | null
6366
mode: Mode
@@ -82,35 +85,41 @@ export const hoverOnSubstep = (
8285
type: 'HOVER_ON_SUBSTEP',
8386
payload: payload,
8487
})
88+
8589
export const selectTerminalItem = (
8690
terminalId: TerminalItemId
8791
): SelectTerminalItemAction => ({
8892
type: 'SELECT_TERMINAL_ITEM',
8993
payload: terminalId,
9094
})
95+
9196
export const hoverOnStep = (
9297
stepId: StepIdType | null | undefined
9398
): HoverOnStepAction => ({
9499
type: 'HOVER_ON_STEP',
95100
payload: stepId,
96101
})
102+
97103
export const hoverOnTerminalItem = (
98104
terminalId: TerminalItemId | null | undefined
99105
): HoverOnTerminalItemAction => ({
100106
type: 'HOVER_ON_TERMINAL_ITEM',
101107
payload: terminalId,
102108
})
109+
103110
export const setWellSelectionLabwareKey = (
104111
labwareName: string | null | undefined
105112
): SetWellSelectionLabwareKeyAction => ({
106113
type: 'SET_WELL_SELECTION_LABWARE_KEY',
107114
payload: labwareName,
108115
})
116+
109117
export const clearWellSelectionLabwareKey =
110118
(): ClearWellSelectionLabwareKeyAction => ({
111119
type: 'CLEAR_WELL_SELECTION_LABWARE_KEY',
112120
payload: null,
113121
})
122+
114123
export const resetSelectStep =
115124
(stepId: StepIdType): ThunkAction<any> =>
116125
(dispatch: ThunkDispatch<any>, getState: GetState) => {
@@ -227,6 +236,7 @@ export const populateForm =
227236
setSelection(formData, dispatch)
228237
resetScrollElements()
229238
}
239+
230240
export const selectStep =
231241
(stepId: StepIdType): ThunkAction<any> =>
232242
(dispatch: ThunkDispatch<any>, getState: GetState) => {
@@ -243,6 +253,7 @@ export const selectStep =
243253
})
244254
setSelection(formData, dispatch)
245255
}
256+
246257
// NOTE(sa, 2020-12-11): this is a thunk so that we can populate the batch edit form with things later
247258
export const selectMultipleSteps =
248259
(
@@ -259,21 +270,7 @@ export const selectMultipleSteps =
259270
}
260271
dispatch(selectStepAction)
261272
}
262-
export const selectMultipleStepsForGroup =
263-
(
264-
stepIds: StepIdType[],
265-
lastSelected: StepIdType
266-
): ThunkAction<SelectMultipleStepsForGroupAction> =>
267-
(dispatch: ThunkDispatch<any>, getState: GetState) => {
268-
const selectStepAction: SelectMultipleStepsForGroupAction = {
269-
type: 'SELECT_MULTIPLE_STEPS_FOR_GROUP',
270-
payload: {
271-
stepIds,
272-
lastSelected,
273-
},
274-
}
275-
dispatch(selectStepAction)
276-
}
273+
277274
export const selectAllSteps =
278275
(): ThunkAction<SelectMultipleStepsAction | AnalyticsEventAction> =>
279276
(
@@ -300,8 +297,10 @@ export const selectAllSteps =
300297
dispatch(analyticsEvent(selectAllStepsEvent))
301298
}
302299
}
300+
303301
export const EXIT_BATCH_EDIT_MODE_BUTTON_PRESS: 'EXIT_BATCH_EDIT_MODE_BUTTON_PRESS' =
304302
'EXIT_BATCH_EDIT_MODE_BUTTON_PRESS'
303+
305304
// todo(mm, 2025-10-31): "deselectAllSteps" is a bit of a misnomer, since this also selects a step.
306305
export const deselectAllSteps =
307306
(

protocol-designer/src/ui/steps/actions/types.ts

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -89,13 +89,6 @@ export interface SelectMultipleStepsAction {
8989
}
9090
}
9191

92-
export interface SelectMultipleStepsForGroupAction {
93-
type: 'SELECT_MULTIPLE_STEPS_FOR_GROUP'
94-
payload: {
95-
stepIds: StepIdType[]
96-
lastSelected: StepIdType
97-
}
98-
}
9992
export type ViewSubstep = StepIdType | null
10093
export interface ToggleViewSubstepAction {
10194
type: 'TOGGLE_VIEW_SUBSTEP'

0 commit comments

Comments
 (0)