-
Notifications
You must be signed in to change notification settings - Fork 118
Multi-profile sweeps and more robust edit flows in point-and-click #6437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
franknoirot
merged 39 commits into
main
from
pierremtb/issue6434-Extend-point-and-click-edit-flows-to-non-variable-sweep-declarations
May 6, 2025
Merged
Changes from 25 commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
acc6d41
WIP: Extend point-and-click edit flows to non-variable sweep declarat…
pierremtb 5119a90
Merge branch 'main' into pierremtb/issue6434-Extend-point-and-click-e…
pierremtb 2c5c59c
Merge branch 'main' into pierremtb/issue6434-Extend-point-and-click-e…
pierremtb 56dbaa1
Merge branch 'main' into pierremtb/issue6434-Extend-point-and-click-e…
pierremtb 8a8678e
Clean up
pierremtb 7c9387d
Clean up more and break out to external hopefully testable function
pierremtb 4450402
Apply the same principles to Revolve
pierremtb c312420
Add variable insertion for revolve and extrude
pierremtb 1f205bb
More clean up ahead of sweep migration
pierremtb f37c3ac
Merge branch 'main' into pierremtb/issue6434-Extend-point-and-click-e…
pierremtb f517bd3
Fix more tests
pierremtb 6b0cdb5
Lint
pierremtb 7cdb181
Remove skips on extrude and revolve
pierremtb 8c7c145
Migrate sweep
pierremtb d756fc6
Fix a few tests
pierremtb f18cf36
Merge branch 'main' into pierremtb/issue6434-Extend-point-and-click-e…
pierremtb 5c766a2
Fix more tests
pierremtb 6b4fae0
Extend point-and-click edit flows to non-variable sweep declarations
pierremtb cfa5d6e
Ensure we can edit all three extrude from sample bracket with test
pierremtb 190701c
Fix all but one test
pierremtb 0cc704a
One more
pierremtb 0e2e811
Merge branch 'main' into pierremtb/issue6434-Extend-point-and-click-e…
pierremtb 7420c63
Fix more
pierremtb fd3b7b7
Another fix
pierremtb 630410e
Another fix
pierremtb 692138c
Update src/lang/modifyAst/addSweep.ts
pierremtb 92f11ff
Big arg naming alignment with KCL, move Loft into the mix for consist…
pierremtb cf0ad74
Fix sweep validation and WIP multi profile tests for all sweeps
pierremtb c632304
Fix tests
pierremtb b568888
Lint
pierremtb be80053
Extend point-and-click edit flows to non-variable sweep declarations
pierremtb 288e46a
Fix lofts
pierremtb 86a6ed1
Another oopsie in global replace
pierremtb 00d2140
Clean up operations.ts
pierremtb c939a9b
Merge branch 'main' into pierremtb/issue6434-Extend-point-and-click-e…
pierremtb 7f060cb
Add Frank's suggestions
pierremtb 6bb8293
Fix up some messssss
pierremtb 2d1410e
Last fix hopefully
pierremtb a9cf5ec
Lint
pierremtb File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -6,6 +6,7 @@ import type { EditorFixture } from '@e2e/playwright/fixtures/editorFixture' | |
| import type { SceneFixture } from '@e2e/playwright/fixtures/sceneFixture' | ||
| import type { ToolbarFixture } from '@e2e/playwright/fixtures/toolbarFixture' | ||
| import { expect, test } from '@e2e/playwright/zoo-test' | ||
| import { bracket } from '@e2e/playwright/fixtures/bracket' | ||
|
|
||
| // test file is for testing point an click code gen functionality that's not sketch mode related | ||
|
|
||
|
|
@@ -75,10 +76,19 @@ test.describe('Point-and-click tests', () => { | |
| await toolbar.extrudeButton.click() | ||
| await cmdBar.expectState({ | ||
| stage: 'arguments', | ||
| currentArgKey: 'distance', | ||
| currentArgKey: 'selection', | ||
| currentArgValue: '', | ||
| headerArguments: { Selection: '', Length: '' }, | ||
| highlightedHeaderArg: 'selection', | ||
| commandName: 'Extrude', | ||
| }) | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.expectState({ | ||
| stage: 'arguments', | ||
| currentArgKey: 'length', | ||
| currentArgValue: '5', | ||
| headerArguments: { Selection: '1 face', Distance: '' }, | ||
| highlightedHeaderArg: 'distance', | ||
| headerArguments: { Selection: '1 face', Length: '' }, | ||
| highlightedHeaderArg: 'length', | ||
| commandName: 'Extrude', | ||
| }) | ||
| await cmdBar.progressCmdBar() | ||
|
|
@@ -88,7 +98,7 @@ test.describe('Point-and-click tests', () => { | |
|
|
||
| await cmdBar.expectState({ | ||
| stage: 'review', | ||
| headerArguments: { Selection: '1 face', Distance: '5' }, | ||
| headerArguments: { Selection: '1 face', Length: '5' }, | ||
| commandName: 'Extrude', | ||
| }) | ||
| await cmdBar.progressCmdBar() | ||
|
|
@@ -97,6 +107,102 @@ test.describe('Point-and-click tests', () => { | |
| }) | ||
| }) | ||
|
|
||
| test('Verify in-pipe extrudes in bracket can be edited', async ({ | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New test to showcase the intent here: edit the extrudes in the real-life onboarding bracket |
||
| tronApp, | ||
| context, | ||
| editor, | ||
| homePage, | ||
| page, | ||
| scene, | ||
| toolbar, | ||
| cmdBar, | ||
| }) => { | ||
| await context.addInitScript((initialCode) => { | ||
| localStorage.setItem('persistCode', initialCode) | ||
| }, bracket) | ||
| await homePage.goToModelingScene() | ||
| await scene.settled(cmdBar) | ||
|
|
||
| await test.step(`Edit first extrude via feature tree`, async () => { | ||
| await (await toolbar.getFeatureTreeOperation('Extrude', 0)).dblclick() | ||
| await cmdBar.expectState({ | ||
| stage: 'arguments', | ||
| currentArgKey: 'length', | ||
| currentArgValue: 'width', | ||
| headerArguments: { | ||
| Length: '5', | ||
| }, | ||
| highlightedHeaderArg: 'length', | ||
| commandName: 'Extrude', | ||
| }) | ||
| await page.keyboard.insertText('width - 0.001') | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.expectState({ | ||
| stage: 'review', | ||
| headerArguments: { | ||
| Length: '4.999', | ||
| }, | ||
| commandName: 'Extrude', | ||
| }) | ||
| await cmdBar.progressCmdBar() | ||
| await editor.expectEditor.toContain('extrude(length = width - 0.001)') | ||
| }) | ||
|
|
||
| await test.step(`Edit second extrude via feature tree`, async () => { | ||
| await (await toolbar.getFeatureTreeOperation('Extrude', 1)).dblclick() | ||
| await cmdBar.expectState({ | ||
| stage: 'arguments', | ||
| currentArgKey: 'length', | ||
| currentArgValue: '-thickness - .01', | ||
| headerArguments: { | ||
| Length: '-0.3949', | ||
| }, | ||
| highlightedHeaderArg: 'length', | ||
| commandName: 'Extrude', | ||
| }) | ||
| await page.keyboard.insertText('-thickness - .01 - 0.001') | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.expectState({ | ||
| stage: 'review', | ||
| headerArguments: { | ||
| Length: '-0.3959', | ||
| }, | ||
| commandName: 'Extrude', | ||
| }) | ||
| await cmdBar.progressCmdBar() | ||
| await editor.expectEditor.toContain( | ||
| 'extrude(length = -thickness - .01 - 0.001)' | ||
| ) | ||
| }) | ||
|
|
||
| await test.step(`Edit third extrude via feature tree`, async () => { | ||
| await (await toolbar.getFeatureTreeOperation('Extrude', 2)).dblclick() | ||
| await cmdBar.expectState({ | ||
| stage: 'arguments', | ||
| currentArgKey: 'length', | ||
| currentArgValue: '-thickness - 0.1', | ||
| headerArguments: { | ||
| Length: '-0.4849', | ||
| }, | ||
| highlightedHeaderArg: 'length', | ||
| commandName: 'Extrude', | ||
| }) | ||
| await page.keyboard.insertText('-thickness - 0.1 - 0.001') | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.expectState({ | ||
| stage: 'review', | ||
| headerArguments: { | ||
| Length: '-0.4859', | ||
| }, | ||
| commandName: 'Extrude', | ||
| }) | ||
| await cmdBar.progressCmdBar() | ||
| await editor.expectEditor.toContain( | ||
| 'extrude(length = -thickness - 0.1 - 0.001)' | ||
| ) | ||
| }) | ||
| }) | ||
|
|
||
| test.describe('verify sketch on chamfer works', () => { | ||
| const _sketchOnAChamfer = | ||
| ( | ||
|
|
@@ -1681,8 +1787,7 @@ sketch002 = startSketchOn(XZ) | |
| testPoint.x - 50, | ||
| testPoint.y | ||
| ) | ||
| const sweepDeclaration = | ||
| 'sweep001 = sweep(profile001, path = sketch002, sectional = false)' | ||
| const sweepDeclaration = 'sweep001 = sweep(profile001, path = sketch002)' | ||
| const editedSweepDeclaration = | ||
| 'sweep001 = sweep(profile001, path = sketch002, sectional = true)' | ||
|
|
||
|
|
@@ -1709,6 +1814,7 @@ sketch002 = startSketchOn(XZ) | |
| stage: 'arguments', | ||
| }) | ||
| await clickOnSketch1() | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.expectState({ | ||
| commandName: 'Sweep', | ||
| currentArgKey: 'trajectory', | ||
|
|
@@ -1722,7 +1828,18 @@ sketch002 = startSketchOn(XZ) | |
| stage: 'arguments', | ||
| }) | ||
| await clickOnSketch2() | ||
| await page.waitForTimeout(500) | ||
| await cmdBar.expectState({ | ||
| commandName: 'Sweep', | ||
| currentArgKey: 'trajectory', | ||
| currentArgValue: '', | ||
| headerArguments: { | ||
| Sectional: '', | ||
| Target: '1 face', | ||
| Trajectory: '', | ||
| }, | ||
| highlightedHeaderArg: 'trajectory', | ||
| stage: 'arguments', | ||
| }) | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.expectState({ | ||
| commandName: 'Sweep', | ||
|
|
@@ -1848,6 +1965,7 @@ sketch002 = startSketchOn(XZ) | |
| stage: 'arguments', | ||
| }) | ||
| await clickOnSketch1() | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.expectState({ | ||
| commandName: 'Sweep', | ||
| currentArgKey: 'trajectory', | ||
|
|
@@ -1861,7 +1979,6 @@ sketch002 = startSketchOn(XZ) | |
| stage: 'arguments', | ||
| }) | ||
| await clickOnSketch2() | ||
| await page.waitForTimeout(500) | ||
| await cmdBar.progressCmdBar() | ||
| await expect( | ||
| page.getByText('Unable to sweep with the current selection. Reason:') | ||
|
|
@@ -3513,6 +3630,7 @@ tag=$rectangleSegmentC002, | |
| await cmdBar.progressCmdBar() | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.progressCmdBar() | ||
| await cmdBar.progressCmdBar() | ||
|
|
||
| const newCodeToFind = `revolve001 = revolve(sketch002, angle = 360, axis = X)` | ||
| expect(editor.expectEditor.toContain(newCodeToFind)).toBeTruthy() | ||
|
|
@@ -3585,6 +3703,7 @@ sketch002 = startSketchOn(extrude001, face = rectangleSegmentA001) | |
| await editor.scrollToText(codeToSelection) | ||
| await page.getByText(codeToSelection).click() | ||
| await toolbar.revolveButton.click() | ||
| await cmdBar.progressCmdBar() | ||
| await page.getByText('Edge', { exact: true }).click() | ||
| const lineCodeToSelection = `angledLine(angle = 0, length = 202.6, tag = $rectangleSegmentA001)` | ||
| await page.getByText(lineCodeToSelection).click() | ||
|
|
@@ -3677,6 +3796,7 @@ sketch002 = startSketchOn(extrude001, face = rectangleSegmentA001) | |
| await page.waitForTimeout(1000) | ||
| await editor.scrollToText(codeToSelection) | ||
| await page.getByText(codeToSelection).click() | ||
| await cmdBar.progressCmdBar() | ||
| await expect.poll(() => page.getByText('AxisOrEdge').count()).toBe(2) | ||
| await page.getByText('Edge', { exact: true }).click() | ||
| const lineCodeToSelection = `length = 2.6` | ||
|
|
||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of the changes here and below are to account for:
skip: trueAnd I took the opportunity to modernize those tests a bit to new fixtures
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice 🔥