Assemblies: UX improvements around foreign file imports by pierremtb · Pull Request #6159 · KittyCAD/modeling-app

pierremtb · 2025-04-04T20:23:08Z

Makes the interaction with non-KCL files in the context of assemblies easier, as a follow up to the initial PR for point-and-click assemblies

Includes:

a fix to show .step files in the file tree (previously only .stp) through more consistent checks around importable files
a test that inserts foreign files in main.kcl, both .step and .sldprt
an automatically generated default value for local name alias in the insert flow, inluding left digit padding as needed
a change in file tree click on foreign files behavior from kcl wrap to prompt to insert into current file
an update to the deep link kcl wrapper moving the whole-module import pattern from the deprecated import() call. Note: I'm still rather unhappy about the kcl wrapper approach that feels somewhat unintuitive to me at least. I'd rather us loading the file into a new project that we'd prompt the use to save somewhere first. But that's outside of the scope here

Does not include:

Mind the next two stacked PRs as well that will be made available after this one:

Assemblies: Load outside files into project via point-and-click #6217
- this includes moving the current Insert button from the side toolbar to the modeling toolbar
Assemblies: Set translate and rotate via point-and-click #6167

Demo:
https://github.com/user-attachments/assets/cca91477-44df-4687-8761-4dc604ceb698

Will eventually fix #6120 Right now the whole loop is there but the codemod doesn't work yet

…rt-for-geometry

Fixes #6152

vercel · 2025-04-04T20:23:12Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
modeling-app	✅ Ready (Inspect)	Visit Preview	Apr 9, 2025 8:40am

qa-wolf · 2025-04-04T20:23:14Z

QA Wolf here! As you write new code it's important that your test coverage is keeping up.
Click here to request test coverage for this PR!

codecov · 2025-04-04T20:44:39Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.01%. Comparing base (ae9d8be) to head (cf34a9d).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6159      +/-   ##
==========================================
- Coverage   85.03%   85.01%   -0.03%     
==========================================
  Files         109      109              
  Lines       44758    44758              
==========================================
- Hits        38062    38050      -12     
- Misses       6696     6708      +12

Flag	Coverage Δ
rust	`85.01% <ø> (-0.03%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

…rt-for-geometry

…etry' into pierremtb/issue6152-UX-improvements-around-foreign-file-imports

@Irev-Dev

Thanks @Irev-Dev for the suggestion

…oreign-file-imports

Irev-Dev

Approving because I know this is all behind a DEV flag, so up to you if you want to fix as part of this PR or follow up.

But essentially,

When importing an obj (maybe applies to others) I get an error straight away, that is fixed just by causing a re-execute
When importing there's no check to see if the local name conflicts with existing variable names.
maybe want to test the above when fixed.

Screenshare.-.2025-04-09.9_06_57.PM.mp4

Irev-Dev · 2025-04-09T10:40:37Z

src/lib/desktop.ts

+export const isRelevantFile = (filename: string): boolean =>
+  RELEVANT_FILE_TYPES.some((ext) => filename.endsWith('.' + ext))


You will probably have a conflict with
#6211

Oooh nice appreciate the heads up. Will get it updated to match in a follow up once it lands!

Irev-Dev · 2025-04-09T10:57:28Z

src/lib/utils.ts

  return sourceRange[2]
 }
+
+export function getInVariableCase(name: string, prefixIfDigit = 'm') {


Don't really understand the name though, the in in particular.

Haha yeah, maybe getStringInVariableCase? I'll think of a better name

pierremtb · 2025-04-09T11:46:32Z

Nice thanks a bunch @Irev-Dev!

When importing an obj (maybe applies to others) I get an error straight away, that is fixed just by causing a re-execute

pretty certain this is related to #5780. will do more testing

When importing there's no check to see if the local name conflicts with existing variable names.

that's a great gotcha thanks for pointing that out

maybe want to test the above when fixed.

yes abosolutely!

I think I will ride the green CI high and merge-as is, will address point 2&3 in a dedicated PR

pierremtb · 2025-04-09T11:47:31Z

Created follow up issue at #6230 under the meta issue

@Irev-Dev

) * WIP: Add point-and-click Import for geometry Will eventually fix #6120 Right now the whole loop is there but the codemod doesn't work yet * Better pathToNOde, log on non-working cm dispatch call * Add workaround to updateModelingState not working * Back to updateModelingState with a skip flag * Better todo * Change working from Import to Insert, cleanups * Sister command in kclCommands to populate file options * Improve path selector * Unsure: move importAstMod to kclCommands onSubmit 😶 * Add e2e test * Clean up for review * Add native file menu entry and test * No await yo lint said so * WIP: UX improvements around foreign file imports Fixes #6152 * @lrev-Dev's suggestion to remove a comment Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch> * Update to scene.settled(cmdBar) * Add partNNN default name for alias * Lint * Lint * Fix unit tests * Add sad path insert test Thanks @Irev-Dev for the suggestion * Add step insert test * Lint * Add test for second foreign import thru file tree click * Add default value for local name alias * Aligning tests * Fix tests * Add padding for filenames starting with a digit --------- Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

* main: #4469 Improve toolbar ux (#5841) Assemblies: UX improvements around foreign file imports (#6159)

pierremtb and others added 19 commits April 2, 2025 17:04

WIP: Add point-and-click Import for geometry

9001006

Will eventually fix #6120 Right now the whole loop is there but the codemod doesn't work yet

Better pathToNOde, log on non-working cm dispatch call

3afcb33

Add workaround to updateModelingState not working

e00f6c2

Back to updateModelingState with a skip flag

2bae5ff

Better todo

79fd435

Update from main

2d08d47

Change working from Import to Insert, cleanups

1bb37fc

Sister command in kclCommands to populate file options

9371860

Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…

69a0198

…rt-for-geometry

Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…

58c87ac

…rt-for-geometry

Improve path selector

9e32ce3

Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…

5d8f7a5

…rt-for-geometry

Unsure: move importAstMod to kclCommands onSubmit 😶

7b7d8b4

Add e2e test

eaf1f09

Clean up for review

8052af8

Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…

efcf164

…rt-for-geometry

Add native file menu entry and test

8fa8b16

No await yo lint said so

5f11d61

WIP: UX improvements around foreign file imports

f36905b

Fixes #6152

pierremtb changed the base branch from main to pierremtb/issue6120-Add-point-and-click-Import-for-geometry April 4, 2025 20:24

pierremtb changed the title ~~Pierremtb/issue6152-UX-improvements-around-foreign-file-imports~~ WIP: UX improvements around foreign file imports Apr 4, 2025

vercel bot deployed to Preview April 4, 2025 20:36 View deployment

pierremtb and others added 4 commits April 7, 2025 08:37

@lrev-Dev's suggestion to remove a comment

4698339

Co-authored-by: Kurt Hutten <k.hutten@protonmail.ch>

Merge branch 'main' into pierremtb/issue6120-Add-point-and-click-Impo…

a932339

…rt-for-geometry

Update to scene.settled(cmdBar)

c5e18ba

Merge branch 'pierremtb/issue6120-Add-point-and-click-Import-for-geom…

ea6eaf5

…etry' into pierremtb/issue6152-UX-improvements-around-foreign-file-imports

vercel bot deployed to Preview April 7, 2025 17:37 View deployment

Add sad path insert test

3c38880

Thanks @Irev-Dev for the suggestion

pierremtb changed the title ~~WIP: UX improvements around foreign file imports~~ UX improvements around foreign file imports Apr 7, 2025

vercel bot deployed to Preview April 7, 2025 23:53 View deployment

Add step insert test

7574663

vercel bot deployed to Preview April 8, 2025 11:21 View deployment

pierremtb and others added 2 commits April 8, 2025 07:21

Merge branch 'main' into pierremtb/issue6152-UX-improvements-around-f…

cd0627b

…oreign-file-imports

Lint

17be7a3

vercel bot deployed to Preview April 8, 2025 11:47 View deployment

Add test for second foreign import thru file tree click

ae72c7c

vercel bot deployed to Preview April 8, 2025 13:27 View deployment

pierremtb changed the title ~~UX improvements around foreign file imports~~ Assemblies: UX improvements around foreign file imports Apr 8, 2025

pierremtb and others added 4 commits April 8, 2025 15:16

Add default value for local name alias

f21c6fe

Merge branch 'main' into pierremtb/issue6152-UX-improvements-around-f…

7d80d30

…oreign-file-imports

Aligning tests

b99a12f

Fix tests

1a538cd

vercel bot deployed to Preview April 8, 2025 19:52 View deployment

Add padding for filenames starting with a digit

2f94ce7

vercel bot deployed to Preview April 8, 2025 20:24 View deployment

pierremtb marked this pull request as ready for review April 9, 2025 08:24

pierremtb requested a review from a team April 9, 2025 08:24

Merge branch 'main' into pierremtb/issue6152-UX-improvements-around-f…

cf34a9d

…oreign-file-imports

vercel bot deployed to Preview April 9, 2025 08:40 View deployment

Irev-Dev approved these changes Apr 9, 2025

View reviewed changes

pierremtb mentioned this pull request Apr 9, 2025

Check for local name alias clashes with extisting var in Insert flow #6230

Closed

pierremtb merged commit e78100e into main Apr 9, 2025
38 checks passed

pierremtb deleted the pierremtb/issue6152-UX-improvements-around-foreign-file-imports branch April 9, 2025 11:48

jacebrowning added a commit that referenced this pull request Apr 9, 2025

Merge branch 'main' into share-snapshots

634cfcb

* main: #4469 Improve toolbar ux (#5841) Assemblies: UX improvements around foreign file imports (#6159)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Assemblies: UX improvements around foreign file imports#6159

Assemblies: UX improvements around foreign file imports#6159
pierremtb merged 39 commits intomainfrom
pierremtb/issue6152-UX-improvements-around-foreign-file-imports

pierremtb commented Apr 4, 2025 •

edited

Loading

Uh oh!

vercel bot commented Apr 4, 2025 •

edited

Loading

Uh oh!

qa-wolf bot commented Apr 4, 2025

Uh oh!

codecov bot commented Apr 4, 2025 •

edited

Loading

Uh oh!

Irev-Dev left a comment •

edited

Loading

Uh oh!

Irev-Dev Apr 9, 2025

Uh oh!

pierremtb Apr 9, 2025

Uh oh!

Irev-Dev Apr 9, 2025

Uh oh!

Irev-Dev Apr 9, 2025

Uh oh!

pierremtb Apr 9, 2025

Uh oh!

pierremtb commented Apr 9, 2025

Uh oh!

pierremtb commented Apr 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

		export const isRelevantFile = (filename: string): boolean =>
		RELEVANT_FILE_TYPES.some((ext) => filename.endsWith('.' + ext))

Conversation

pierremtb commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vercel bot commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

qa-wolf bot commented Apr 4, 2025

Uh oh!

codecov bot commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Irev-Dev left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Irev-Dev Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

pierremtb Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Irev-Dev Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

Irev-Dev Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

pierremtb Apr 9, 2025

Choose a reason for hiding this comment

Uh oh!

pierremtb commented Apr 9, 2025

Uh oh!

pierremtb commented Apr 9, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

pierremtb commented Apr 4, 2025 •

edited

Loading

vercel bot commented Apr 4, 2025 •

edited

Loading

codecov bot commented Apr 4, 2025 •

edited

Loading

Irev-Dev left a comment •

edited

Loading