Skip to content

[Bug] fix some UI friction from imports#6139

Merged
Irev-Dev merged 5 commits intomainfrom
kurt-friction-with-imports
Apr 4, 2025
Merged

[Bug] fix some UI friction from imports#6139
Irev-Dev merged 5 commits intomainfrom
kurt-friction-with-imports

Conversation

@Irev-Dev
Copy link
Contributor

@Irev-Dev Irev-Dev commented Apr 4, 2025

Screenshare.-.2025-04-04.3_43_30.PM.mp4

@qa-wolf
Copy link

qa-wolf bot commented Apr 4, 2025

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!

@vercel
Copy link

vercel bot commented Apr 4, 2025

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

Name Status Preview Updated (UTC)
modeling-app ✅ Ready (Inspect) Visit Preview Apr 4, 2025 7:00am

@codecov
Copy link

codecov bot commented Apr 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 85.37%. Comparing base (bdec611) to head (fe863bc).
Report is 2 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #6139   +/-   ##
=======================================
  Coverage   85.37%   85.37%           
=======================================
  Files         110      110           
  Lines       44215    44215           
=======================================
  Hits        37750    37750           
  Misses       6465     6465           
Flag Coverage Δ
rust 85.37% <ø> (ø)

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.

if (fileIndex !== 0) {
const importDetails =
kclManager.execState.filenames[fileIndex]
if (importDetails?.type === 'Local') {
Copy link
Contributor Author

@Irev-Dev Irev-Dev Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wanted to check with the kcl oracles if I should checking anything else here, there's main and std but main is the happy path, and we std is not ever geometry right?

there might one day be url or similar, but can probably update at the time?

Copy link
Contributor

@jtran jtran Apr 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's correct. I don't think anything restricts std from having geometry, but we don't currently.

If you'd like the addition of a new case to fail tsc, you can make an exhaustive check like this:

const _exhaustiveCheck: never = op.group

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@Irev-Dev Irev-Dev requested a review from a team April 4, 2025 05:38
@Irev-Dev Irev-Dev merged commit f1e9515 into main Apr 4, 2025
38 checks passed
@Irev-Dev Irev-Dev deleted the kurt-friction-with-imports branch April 4, 2025 08:38
modeling-app-github-app bot pushed a commit that referenced this pull request Apr 4, 2025
* fix some UI friction from imports

* add test

* console

* Jon's comments
modeling-app-github-app bot pushed a commit that referenced this pull request Apr 4, 2025
* fix some UI friction from imports

* add test

* console

* Jon's comments
modeling-app-github-app bot pushed a commit that referenced this pull request Apr 4, 2025
* fix some UI friction from imports

* add test

* console

* Jon's comments
modeling-app-github-app bot pushed a commit that referenced this pull request Apr 4, 2025
* fix some UI friction from imports

* add test

* console

* Jon's comments
Comment on lines +193 to +194
const fileIndex = getModuleId(extrusion.codeRef.range)
if (fileIndex !== 0) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One nitpick. We can use isTopLevelModule() here too.

Suggested change
const fileIndex = getModuleId(extrusion.codeRef.range)
if (fileIndex !== 0) {
if (!isTopLevelModule(extrusion.codeRef.range)) {
const fileIndex = getModuleId(extrusion.codeRef.range)

modeling-app-github-app bot pushed a commit that referenced this pull request Apr 4, 2025
* fix some UI friction from imports

* add test

* console

* Jon's comments
jessfraz added a commit that referenced this pull request Apr 5, 2025
* origin/main: (26 commits)
  attempt to import win-ca on windows (#6136)
  Upgrade e2e-tests windows runner from 4 cores to 8 (#6166)
  Follow-up fixes after bearing sample rename (#6164)
  Add test for #5799: "Only showing axis planes when there are no errors" (#6007)
  Wait for export button to make test more reliable (#6143)
  sketching on a mirror2d thats been extruded fixed! (#6149)
  Bump vite from 5.4.16 to 5.4.17 in /packages/codemirror-lang-kcl in the security group (#6150)
  Bump vite from 5.4.16 to 5.4.17 in the security group (#6151)
  Update all KCL-Samples to be more ME friendly (#6132)
  Shorten feedback cycle for legitimate failures (#6146)
  Remove the camera projection toggle from the UI (#6077)
  Use all available CPUs to run tests on CI (#6138)
  [fix] Get rid of risky useEffect in restart onboarding flow (#6133)
  Feature: Traditional menu actions in desktop application part II (#6030)
  [Bug] fix some UI friction from imports (#6139)
  Use scene fixture to make test more reliable on macOS (#6140)
  Fix: function composition during playwright setup created a massive page.reload loop (#6137)
  Alternative way to make appMachine spawned children type safe (#5890)
  [BUG] mutate ast to keep comments for pipe split ast-mod (#6128)
  Rename the app to Zoo Design Studio (#5974)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants