Skip to content

Commit 5f7a75a

Browse files
authored
#7408 Can not pick sketch plane using the feature tree and related improvements (#7609)
* Add ability to pick default plane in feature tree in 'Sketch no face' mode * add ability to select deoffset plane where starting a new sketch * use selectDefaultSketchPlane * refactor: remove some duplication * warning cleanups * feature tree items selectable depedngin on no face sketch mode * lint * fix small jump because of border:none when going into and back from 'No face sketch' mode * grey out items other than offset planes in 'No face sketch' mode * start sketching on plane in context menu * sketch on offset plane with context menu * add ability to right click on default plane and start sketch on it * default planes in feature tree should be selectable because of right click context menu * add right click Start sketch option for selected plane on the canvas * selectDefaultSketchPlane returns error now * circular deps * move select functions to lib/selections.ts to avoid circular deps * add test for clicking on feature tree after starting a new sketch * graphite suggestion * fix bug of not being able to create offset plane using another offset plane with command bar * add ability to select default plane on feature when going through the Offset plane command bar flow
1 parent 01230b0 commit 5f7a75a

File tree

7 files changed

+442
-150
lines changed

7 files changed

+442
-150
lines changed

e2e/playwright/sketch-tests.spec.ts

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,68 @@ sketch001 = startProfile(sketch002, at = [12.34, -12.34])
187187
page.getByRole('button', { name: 'Start Sketch' })
188188
).toBeVisible()
189189
})
190+
191+
test('Can select planes in Feature Tree after Start Sketch', async ({
192+
page,
193+
homePage,
194+
toolbar,
195+
editor,
196+
}) => {
197+
// Load the app with empty code
198+
await page.addInitScript(async () => {
199+
localStorage.setItem(
200+
'persistCode',
201+
`plane001 = offsetPlane(XZ, offset = 5)`
202+
)
203+
})
204+
205+
await page.setBodyDimensions({ width: 1200, height: 500 })
206+
207+
await homePage.goToModelingScene()
208+
209+
await test.step('Click Start Sketch button', async () => {
210+
await page.getByRole('button', { name: 'Start Sketch' }).click()
211+
await expect(
212+
page.getByRole('button', { name: 'Exit Sketch' })
213+
).toBeVisible()
214+
await expect(page.getByText('select a plane or face')).toBeVisible()
215+
})
216+
217+
await test.step('Open feature tree and select Front plane (XZ)', async () => {
218+
await toolbar.openFeatureTreePane()
219+
220+
await page.getByRole('button', { name: 'Front plane' }).click()
221+
222+
await page.waitForTimeout(600)
223+
224+
await expect(toolbar.lineBtn).toBeEnabled()
225+
await editor.expectEditor.toContain('startSketchOn(XZ)')
226+
227+
await page.getByRole('button', { name: 'Exit Sketch' }).click()
228+
await expect(
229+
page.getByRole('button', { name: 'Start Sketch' })
230+
).toBeVisible()
231+
})
232+
233+
await test.step('Click Start Sketch button again', async () => {
234+
await page.getByRole('button', { name: 'Start Sketch' }).click()
235+
await expect(
236+
page.getByRole('button', { name: 'Exit Sketch' })
237+
).toBeVisible()
238+
})
239+
240+
await test.step('Select the offset plane', async () => {
241+
await toolbar.openFeatureTreePane()
242+
243+
await page.getByRole('button', { name: 'Offset plane' }).click()
244+
245+
await page.waitForTimeout(600)
246+
247+
await expect(toolbar.lineBtn).toBeEnabled()
248+
await editor.expectEditor.toContain('startSketchOn(plane001)')
249+
})
250+
})
251+
190252
test('Can edit segments by dragging their handles', () => {
191253
const doEditSegmentsByDraggingHandle = async (
192254
page: Page,

src/components/ModelingSidebar/ModelingPanes/FeatureTreePane.tsx

Lines changed: 122 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,12 @@ import {
2424
getOperationVariableName,
2525
stdLibMap,
2626
} from '@src/lib/operations'
27-
import { editorManager, kclManager, rustContext } from '@src/lib/singletons'
27+
import {
28+
editorManager,
29+
kclManager,
30+
rustContext,
31+
sceneInfra,
32+
} from '@src/lib/singletons'
2833
import {
2934
featureTreeMachine,
3035
featureTreeMachineDefaultContext,
@@ -34,11 +39,20 @@ import {
3439
kclEditorActor,
3540
selectionEventSelector,
3641
} from '@src/machines/kclEditorMachine'
42+
import type { Plane } from '@rust/kcl-lib/bindings/Artifact'
43+
import {
44+
selectDefaultSketchPlane,
45+
selectOffsetSketchPlane,
46+
} from '@src/lib/selections'
47+
import type { DefaultPlaneStr } from '@src/lib/planes'
3748

3849
export const FeatureTreePane = () => {
3950
const isEditorMounted = useSelector(kclEditorActor, editorIsMountedSelector)
4051
const lastSelectionEvent = useSelector(kclEditorActor, selectionEventSelector)
4152
const { send: modelingSend, state: modelingState } = useModelingContext()
53+
54+
const sketchNoFace = modelingState.matches('Sketch no face')
55+
4256
// eslint-disable-next-line @typescript-eslint/no-unused-vars
4357
const [_featureTreeState, featureTreeSend] = useMachine(
4458
featureTreeMachine.provide({
@@ -195,6 +209,7 @@ export const FeatureTreePane = () => {
195209
key={key}
196210
item={operation}
197211
send={featureTreeSend}
212+
sketchNoFace={sketchNoFace}
198213
/>
199214
)
200215
})}
@@ -251,6 +266,7 @@ const OperationItemWrapper = ({
251266
customSuffix,
252267
className,
253268
selectable = true,
269+
greyedOut = false,
254270
...props
255271
}: React.HTMLAttributes<HTMLButtonElement> & {
256272
icon: CustomIconName
@@ -262,18 +278,19 @@ const OperationItemWrapper = ({
262278
menuItems?: ComponentProps<typeof ContextMenu>['items']
263279
errors?: Diagnostic[]
264280
selectable?: boolean
281+
greyedOut?: boolean
265282
}) => {
266283
const menuRef = useRef<HTMLDivElement>(null)
267284

268285
return (
269286
<div
270287
ref={menuRef}
271-
className={`flex select-none items-center group/item my-0 py-0.5 px-1 ${selectable ? 'focus-within:bg-primary/10 hover:bg-primary/5' : ''}`}
288+
className={`flex select-none items-center group/item my-0 py-0.5 px-1 ${selectable ? 'focus-within:bg-primary/10 hover:bg-primary/5' : ''} ${greyedOut ? 'opacity-50 cursor-not-allowed' : ''}`}
272289
data-testid="feature-tree-operation-item"
273290
>
274291
<button
275292
{...props}
276-
className={`reset !py-0.5 !px-1 flex-1 flex items-center gap-2 text-left text-base ${selectable ? 'border-transparent dark:border-transparent' : 'border-none cursor-default'} ${className}`}
293+
className={`reset !py-0.5 !px-1 flex-1 flex items-center gap-2 text-left text-base ${selectable ? 'border-transparent dark:border-transparent' : '!border-transparent cursor-default'} ${className}`}
277294
>
278295
<CustomIcon name={icon} className="w-5 h-5 block" />
279296
<div className="flex flex-1 items-baseline align-baseline">
@@ -311,6 +328,7 @@ const OperationItemWrapper = ({
311328
const OperationItem = (props: {
312329
item: Operation
313330
send: Prop<Actor<typeof featureTreeMachine>, 'send'>
331+
sketchNoFace: boolean
314332
}) => {
315333
const kclContext = useKclContext()
316334
const name = getOperationLabel(props.item)
@@ -343,15 +361,22 @@ const OperationItem = (props: {
343361
}, [kclContext.diagnostics.length])
344362

345363
function selectOperation() {
346-
if (props.item.type === 'GroupEnd') {
347-
return
364+
if (props.sketchNoFace) {
365+
if (isOffsetPlane(props.item)) {
366+
const artifact = findOperationArtifact(props.item)
367+
void selectOffsetSketchPlane(artifact)
368+
}
369+
} else {
370+
if (props.item.type === 'GroupEnd') {
371+
return
372+
}
373+
props.send({
374+
type: 'selectOperation',
375+
data: {
376+
targetSourceRange: sourceRangeFromRust(props.item.sourceRange),
377+
},
378+
})
348379
}
349-
props.send({
350-
type: 'selectOperation',
351-
data: {
352-
targetSourceRange: sourceRangeFromRust(props.item.sourceRange),
353-
},
354-
})
355380
}
356381

357382
/**
@@ -432,6 +457,20 @@ const OperationItem = (props: {
432457
}
433458
}
434459

460+
function startSketchOnOffsetPlane() {
461+
if (isOffsetPlane(props.item)) {
462+
const artifact = findOperationArtifact(props.item)
463+
if (artifact?.id) {
464+
sceneInfra.modelingSend({
465+
type: 'Enter sketch',
466+
data: { forceNewSketch: true },
467+
})
468+
469+
void selectOffsetSketchPlane(artifact)
470+
}
471+
}
472+
}
473+
435474
const menuItems = useMemo(
436475
() => [
437476
<ContextMenuItem
@@ -477,6 +516,13 @@ const OperationItem = (props: {
477516
</ContextMenuItem>,
478517
]
479518
: []),
519+
...(isOffsetPlane(props.item)
520+
? [
521+
<ContextMenuItem onClick={startSketchOnOffsetPlane}>
522+
Start Sketch
523+
</ContextMenuItem>,
524+
]
525+
: []),
480526
...(props.item.type === 'StdLibCall' ||
481527
props.item.type === 'VariableDeclaration'
482528
? [
@@ -550,22 +596,63 @@ const OperationItem = (props: {
550596
[props.item, props.send]
551597
)
552598

599+
const enabled = !props.sketchNoFace || isOffsetPlane(props.item)
600+
553601
return (
554602
<OperationItemWrapper
603+
selectable={enabled}
555604
icon={getOperationIcon(props.item)}
556605
name={name}
557606
variableName={variableName}
558607
valueDetail={valueDetail}
559608
menuItems={menuItems}
560609
onClick={selectOperation}
561-
onDoubleClick={enterEditFlow}
610+
onDoubleClick={props.sketchNoFace ? undefined : enterEditFlow} // no double click in "Sketch no face" mode
562611
errors={errors}
612+
greyedOut={!enabled}
563613
/>
564614
)
565615
}
566616

567617
const DefaultPlanes = () => {
568618
const { state: modelingState, send } = useModelingContext()
619+
const sketchNoFace = modelingState.matches('Sketch no face')
620+
621+
const onClickPlane = useCallback(
622+
(planeId: string) => {
623+
if (sketchNoFace) {
624+
selectDefaultSketchPlane(planeId)
625+
} else {
626+
const foundDefaultPlane =
627+
rustContext.defaultPlanes !== null &&
628+
Object.entries(rustContext.defaultPlanes).find(
629+
([, plane]) => plane === planeId
630+
)
631+
if (foundDefaultPlane) {
632+
send({
633+
type: 'Set selection',
634+
data: {
635+
selectionType: 'defaultPlaneSelection',
636+
selection: {
637+
name: foundDefaultPlane[0] as DefaultPlaneStr,
638+
id: planeId,
639+
},
640+
},
641+
})
642+
}
643+
}
644+
},
645+
[sketchNoFace]
646+
)
647+
648+
const startSketchOnDefaultPlane = useCallback((planeId: string) => {
649+
sceneInfra.modelingSend({
650+
type: 'Enter sketch',
651+
data: { forceNewSketch: true },
652+
})
653+
654+
selectDefaultSketchPlane(planeId)
655+
}, [])
569656

570657
const defaultPlanes = rustContext.defaultPlanes
571658
if (!defaultPlanes) return null
@@ -603,7 +690,15 @@ const DefaultPlanes = () => {
603690
customSuffix={plane.customSuffix}
604691
icon={'plane'}
605692
name={plane.name}
606-
selectable={false}
693+
selectable={true}
694+
onClick={() => onClickPlane(plane.id)}
695+
menuItems={[
696+
<ContextMenuItem
697+
onClick={() => startSketchOnDefaultPlane(plane.id)}
698+
>
699+
Start Sketch
700+
</ContextMenuItem>,
701+
]}
607702
visibilityToggle={{
608703
visible: modelingState.context.defaultPlaneVisibility[plane.key],
609704
onVisibilityChange: () => {
@@ -620,3 +715,17 @@ const DefaultPlanes = () => {
620715
</div>
621716
)
622717
}
718+
719+
type StdLibCallOp = Extract<Operation, { type: 'StdLibCall' }>
720+
721+
const isOffsetPlane = (item: Operation): item is StdLibCallOp => {
722+
return item.type === 'StdLibCall' && item.name === 'offsetPlane'
723+
}
724+
725+
const findOperationArtifact = (item: StdLibCallOp) => {
726+
const nodePath = JSON.stringify(item.nodePath)
727+
const artifact = [...kclManager.artifactGraph.values()].find(
728+
(a) => JSON.stringify((a as Plane).codeRef?.nodePath) === nodePath
729+
)
730+
return artifact
731+
}

src/components/ViewControlMenu.tsx

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,13 +10,22 @@ import {
1010
import { useModelingContext } from '@src/hooks/useModelingContext'
1111
import type { AxisNames } from '@src/lib/constants'
1212
import { VIEW_NAMES_SEMANTIC } from '@src/lib/constants'
13-
import { sceneInfra } from '@src/lib/singletons'
14-
import { reportRejection } from '@src/lib/trap'
13+
import { kclManager, sceneInfra } from '@src/lib/singletons'
14+
import { err, reportRejection } from '@src/lib/trap'
1515
import { useSettings } from '@src/lib/singletons'
1616
import { resetCameraPosition } from '@src/lib/resetCameraPosition'
17+
import type { Selections } from '@src/lib/selections'
18+
import {
19+
selectDefaultSketchPlane,
20+
selectOffsetSketchPlane,
21+
} from '@src/lib/selections'
1722

1823
export function useViewControlMenuItems() {
1924
const { state: modelingState, send: modelingSend } = useModelingContext()
25+
const selectedPlaneId = getCurrentPlaneId(
26+
modelingState.context.selectionRanges
27+
)
28+
2029
const settings = useSettings()
2130
const shouldLockView =
2231
modelingState.matches('Sketch') &&
@@ -56,9 +65,35 @@ export function useViewControlMenuItems() {
5665
Center view on selection
5766
</ContextMenuItem>,
5867
<ContextMenuDivider />,
68+
<ContextMenuItem
69+
onClick={() => {
70+
if (selectedPlaneId) {
71+
sceneInfra.modelingSend({
72+
type: 'Enter sketch',
73+
data: { forceNewSketch: true },
74+
})
75+
76+
const defaultSketchPlaneSelected =
77+
selectDefaultSketchPlane(selectedPlaneId)
78+
if (
79+
!err(defaultSketchPlaneSelected) &&
80+
defaultSketchPlaneSelected
81+
) {
82+
return
83+
}
84+
85+
const artifact = kclManager.artifactGraph.get(selectedPlaneId)
86+
void selectOffsetSketchPlane(artifact)
87+
}
88+
}}
89+
disabled={!selectedPlaneId}
90+
>
91+
Start sketch on selection
92+
</ContextMenuItem>,
93+
<ContextMenuDivider />,
5994
<ContextMenuItemRefresh />,
6095
],
61-
[VIEW_NAMES_SEMANTIC, shouldLockView]
96+
[VIEW_NAMES_SEMANTIC, shouldLockView, selectedPlaneId]
6297
)
6398
return menuItems
6499
}
@@ -77,3 +112,21 @@ export function ViewControlContextMenu({
77112
/>
78113
)
79114
}
115+
116+
function getCurrentPlaneId(selectionRanges: Selections): string | null {
117+
const defaultPlane = selectionRanges.otherSelections.find(
118+
(selection) => typeof selection === 'object' && 'name' in selection
119+
)
120+
if (defaultPlane) {
121+
return defaultPlane.id
122+
}
123+
124+
const planeSelection = selectionRanges.graphSelections.find(
125+
(selection) => selection.artifact?.type === 'plane'
126+
)
127+
if (planeSelection) {
128+
return planeSelection.artifact?.id || null
129+
}
130+
131+
return null
132+
}

0 commit comments

Comments
 (0)