Allow point-and-click Offset Plane to work from sweep faces #7882
Conversation
Edit flows not working yet Will eventually close #7555
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| @@ -381,26 +381,23 @@ test.describe('Feature Tree pane', () => { | |||
| await cmdBar.expectState({ | |||
| commandName: 'Offset plane', | |||
| stage: 'arguments', | |||
| currentArgKey: 'distance', | |||
| currentArgKey: 'offset', | |||
There was a problem hiding this comment.
Renamed this to be more in line with KCL
| const xz = await toolbar.getFeatureTreeOperation('Front plane', 0) | ||
| await xz.click() |
There was a problem hiding this comment.
And would like to keep these two tests as they aren't px dependent anymore
| /** | ||
| * Append an offset plane to the AST | ||
| */ | ||
| export function addOffsetPlane({ |
There was a problem hiding this comment.
Moved to faces.ts with the addShell codemod since they will share a few utils
| ast: modifiedAst, | ||
| call, | ||
| pathToEdit: nodeToEdit, | ||
| pathIfNewPipe: undefined, |
There was a problem hiding this comment.
No pipe expr for offset planes for now
src/lang/modifyAst/faces.ts
Outdated
| const result = buildSolidsAndFacesExprs( | ||
| faces, | ||
| artifactGraph, |
There was a problem hiding this comment.
Making it a lil nicer for shell as well
src/lib/operations.ts
Outdated
|
|
||
| console.log('Offset plane operation:', operation) | ||
|
|
||
| // TODO: replace with util function when available |
There was a problem hiding this comment.
Function from #7819
…ane-offset-from-a-face-in-point-and-click
…ane-offset-from-a-face-in-point-and-click
pierremtb
changed the title
pierremtb
changed the title
…ane-offset-from-a-face-in-point-and-click
…ane-offset-from-a-face-in-point-and-click
…ane-offset-from-a-face-in-point-and-click
|
amazing work mate!
|
…ane-offset-from-a-face-in-point-and-click
…ane-offset-from-a-face-in-point-and-click
|
@max-mrgrsk Thank you! I fixed the first bullet, and for the second one I created #8068 to track it |
There was a problem hiding this comment.
Works great for me, aside from chamfer faces, which you've already captured an issue for. Great job, the code looks clean and well-thought through. I had a slight urge to suggest more generic approaches to the utility functions in faces.test.ts but I think it's good to hold off on that until it's actually used.
Big unlock for users, way to go!
| // 2. Prepare unlabeled and labeled arguments | ||
| let planeExpr: Expr | undefined | ||
| const hasFaceToOffset = plane.graphSelections.some( | ||
| (sel) => sel.artifact?.type === 'cap' || sel.artifact?.type === 'wall' |
There was a problem hiding this comment.
Here I thought I had you noticing that chamfer faces weren't working! Nice
pierremtb
deleted the
pierremtb/issue7555-Add-ability-to-define-plane-offset-from-a-face-in-point-and-click
branch
Closes #7555 and #7883
Refactors the codemod the same way we did sweeps, transforms, helix, shell as part of https://github.com/KittyCAD/modeling-app/milestone/4, with non-playwright test cases for default planes, offset plane on offset plane, and offset plane on cap.
Two things to resolve before merging:
the rust side could be cleaned up now that we have the plane IDsthe pre-selection gotcha for planes listed in the commentsDemo:
Screen.Recording.2025-07-23.at.2.53.37.PM.mov