feature: Project Explorer update, first pass. #7656

nadr0 · 2025-07-01T19:31:44Z

#7592 got out of hand with the optimizations with little planning on my end.

Opening a fresh PR without this to see if this passes to do my optimizations within another body of work.

…me clean up

…ponents

… logic was not taking parent into account

…, wasting time

…d moust events

…e ref

src/components/Explorer/ProjectExplorer.tsx

nadr0 · 2025-07-01T22:10:52Z

@jacebrowning Whenever this gets reviewed and approved I'll need to disable these two tests

I've disabled the useFileSystesmWatcher within the modeling page. These tests would be N/A until we enable the feature again in the future.

jacebrowning · 2025-07-02T13:14:35Z

I've disabled the useFileSystesmWatcher within the modeling page. These tests would be N/A until we enable the feature again in the future.

@nadr0 OK, please create a GitHub issue to drop in the form to disable the tests. Or, if the "future" where we enable this is a long ways out, consider deleting the tests. They'll always be available in the Git history.

franknoirot · 2025-07-09T15:01:39Z

Working through a code review in a bit, but here are a few initial UX thoughts @nadr0

Seem to have lost the ability to hit Enter and start renaming the file or folder under the cursor. This is also the behavior of VS Code as well. I see now that you have it opening files, hmmm could Shift + Enter be used for either rename or open file? I just want some keyboard shortcut for rename and it used to be Enter I guess.
Minor UX polish for future: Left arrow while cursor is on a file within a folder jumps the cursor up to the folder in VS Code

src/lib/project.d.ts

andrewvarga · 2025-07-09T15:14:24Z

src/lib/paths.ts

+/**
+ * Don't pass a folder path, only files with extensions for best results.
+ */
+export const alwaysEndFileWithEXT = (


Maybe a slightly easier name would be enforceFileEXT.
Also if ext' is null I would expect the function to return filePathinstead ofnull`. I see that null is checked agains in ProjectExplorer so that would need to be changed as well but maybe worth it? In the odd case of the file not having an extension, it could still be renamed without having an extension.

It depends on who we want to have the burden of a check? This comes down to whether we support base cases?

const originalExt = getEXTWithPeriod(name) const fileNameForcedWithOriginalExt = enforceFileEXT( requestedName, originalExt ) if (!fileNameForcedWithOriginalExt) { // TODO: OH NO! return }

I did this because these are paired together, when you try to get the originalExtension from something you may not get that extension so if you attempt to enforce it it could fail. That is why there is a null check.

We really always want extensions in the codebase.

andrewvarga · 2025-07-09T15:34:46Z

src/machines/systemIO/systemIOMachineDesktop.ts

+        } catch (e) {
+          console.error(e)
+        }
+        await window.electron.writeFile(input.requestedAbsolutePath, '')


I think writeFile can throw so maybe it's worth putting inside the try block as well?

This is already caught properly, within the systemIOMachine.ts. Anything that is explicitly not try/catched will be caught and handled with the onError workflow of the actor.

The reason why I try catch the thing above is if you window.electron.stat something and it doesn't exist it will throw an error which is what we want. We want it to say "Yeah you don't have a file" to make sure we can then do the create file.

We don't always need to try/catch handle every statement since the parent workflow in the machine will handle the onError.

invoke: { id: SystemIOMachineActors.createBlankFile, src: SystemIOMachineActors.createBlankFile, input: ({ context, event, self }) => { assertEvent(event, SystemIOMachineEvents.createBlankFile) return { context, requestedAbsolutePath: event.data.requestedAbsolutePath, rootContext: self.system.get('root').getSnapshot().context, } }, onDone: { target: SystemIOMachineStates.readingFolders, actions: [SystemIOMachineActions.toastSuccess], }, onError: { <--- Handled!! target: SystemIOMachineStates.idle, actions: [SystemIOMachineActions.toastError], <-- will toast error! },

andrewvarga · 2025-07-09T15:35:11Z

src/machines/systemIO/systemIOMachineDesktop.ts

+            console.error(e)
+          }
+        }
+        await window.electron.mkdir(input.requestedAbsolutePath, {


Similar here, or maybe there is a reason it is outside the try-catch ?

Same reason as above, the try catch is handled on the systemIOMachine.ts onError definition of the actor being invoked.

andrewvarga · 2025-07-09T15:36:22Z

src/components/ContextMenu.tsx

@@ -43,6 +45,9 @@ export function ContextMenu({
  })
  const handleContextMenu = useCallback(
    (e: globalThis.MouseEvent) => {
+      if (callback) {


short one liner I usually use is callback?.(e) instead of the if but just mentioning it, it may be less readable.

I don't think we have settled this debate internally of shorthands.

if (something) doSomething() yup && yup()

^-- stuff like this

I think it is less readable seeing callback?.(e). Curious if anyone has strong opinions on this?

andrewvarga · 2025-07-09T16:00:14Z

src/components/Explorer/utils.ts

+  list.push(content)
+
+  // if a FileEntry has no children stop
+  if (f.children === null) {


probably doesn't matter but in cases like this I just usually check for falsiness, that handles if for any reasons children becomes undefined

The type is of children: Array<FileEntry> | null. I kinda want stuff to shit the bed if the value is actually undefined?

If we are worried about this we may need some runtime verification on the types to ensure that object is generated without any undefined.

andrewvarga · 2025-07-09T16:17:09Z

Things seem to be working still, I just noticed a few small differences compared to main:

When having a file selected and pressing Enter I get an orange outline around the Filetree pane, not sure if that's on purpose?
Also when having a file selected and then pressing Delete / Backspace I'm getting the same plus a message that the selection cannot be deleted. On main nothing happens (but no error message either).

But it's good that the misleading hotkeys for Enter and ctrl + Del are deleted from the context menu since those didn't work on main anyway.

andrewvarga · 2025-07-09T16:21:32Z

Working through a code review in a bit, but here are a few initial UX thoughts @nadr0

Seem to have lost the ability to hit Enter and start renaming the file or folder under the cursor. This is also the behavior of VS Code as well. I see now that you have it opening files, hmmm could Shift + Enter be used for either rename or open file? I just want some keyboard shortcut for rename and it used to be Enter I guess.

Minor UX polish for future: Left arrow while cursor is on a file within a folder jumps the cursor up to the folder in VS Code

I was going to mention that but it seems even on main it's not possible anymore to rename the files with enter?

nadr0 · 2025-07-23T14:43:06Z

Working through a code review in a bit, but here are a few initial UX thoughts @nadr0

Seem to have lost the ability to hit Enter and start renaming the file or folder under the cursor. This is also the behavior of VS Code as well. I see now that you have it opening files, hmmm could Shift + Enter be used for either rename or open file? I just want some keyboard shortcut for rename and it used to be Enter I guess.

Minor UX polish for future: Left arrow while cursor is on a file within a folder jumps the cursor up to the folder in VS Code

In VS Code when pressing enter opens it opens the file. I've opted for this as well because when pressing enter on folders it opens the folders. I didn't want enter on filenames to be rename but enter on folders to open.
I see, when you are in the first file of the folder it can jump up, gotcha.

Going through comments, i'll come up with a TODO list for this pass of review.

nadr0 · 2025-07-23T18:34:50Z

Things seem to be working still, I just noticed a few small differences compared to main:

When having a file selected and pressing Enter I get an orange outline around the Filetree pane, not sure if that's on purpose? Also when having a file selected and then pressing Delete / Backspace I'm getting the same plus a message that the selection cannot be deleted. On main nothing happens (but no error message either).

But it's good that the misleading hotkeys for Enter and ctrl + Del are deleted from the context menu since those didn't work on main anyway.

The orange color itself is not intentional but the feature is required. I need to focus that area to use the keyboard inputs. I need an active focus on that element.

I won't if that error message is a bug with another part of the system not clearing your selection and having deletion there. I do not have any deletion callback triggered when pressing the delete key on the board.

nadr0 · 2025-07-23T18:35:52Z

.github/workflows/unit-tests.yml

@@ -97,48 +97,3 @@ jobs:
          git commit -am "Look at this (photo)Graph *in the voice of Nickelback*" || true
          git push
          git push origin ${{ github.head_ref }}
-
-  npm-test-unit-components:


All jest unit test component stuff is merged into vitest since it is compatible and supports more of our repository level configurations.

nadr0 · 2025-07-23T18:58:48Z

package.json

@@ -134,8 +134,7 @@
    "test": "vitest --mode=development",
    "test:rust": "(cd rust && just test && just lint)",
    "test:snapshots": "cross-env TARGET=web NODE_ENV=development playwright test --config=playwright.config.ts --grep=@snapshot --trace=on",
-    "test:unit": "vitest run --mode=development --exclude **/jest-component-unit-tests/*",


Again, deleted the jest runtime stuff since vitest automatically supports the component testing as is.

nadr0 · 2025-07-23T19:24:35Z

src/components/ModelingPageProvider.tsx

@@ -0,0 +1,175 @@
+import React, { useEffect, useMemo } from 'react'


This file could be migrated into another files in the future.

It is useful at the moment since it is the largest component when loading the modeling page. It allows us to initialize a few things.

I'd say okay for the time being.

nadr0 · 2025-07-23T19:26:01Z

src/components/billing.test.tsx

@@ -0,0 +1,347 @@
+// @ts-nocheck


Since jest had a different configuration setup this file is failing several static checkers. The configurations that were used in jest are different from our repository level configurations.

I've added some no checks to make the static checker happy. Any new .test.tsx will be good though.

…first file and press left arrow

nadr0 · 2025-07-23T19:50:14Z

I've implemented the functionality that will close your current folder if you are in a folder and in the first position of the folder. It moves up and then closes the folder like VSCode.

nadr0 added 30 commits June 12, 2025 15:57

chore: saving off skeleton progress... need to write unit tests do so…

fcf584e

…me clean up

chore: big cleanup, typing, adding comments, better structure

320a7e1

chore: moved selected row and opened rows up a level in the react com…

c1e0206

…ponents

feature: button to collapse all folders

a5768a0

feat: implemented projet explorer refresh

4983c54

feat: sort was not sorting folders and files at each level, selection…

24d5bbd

… logic was not taking parent into account

chore: unable to implement insert at index, need to come back to this…

c8bba25

…, wasting time

chore: css pointer and hover

23b12de

chore: adding custom folder icon

e45eb57

chore: adding opened icon

746fbcb

feat: adding line for indentation visual

5106e3f

fix: react key warning

424d2aa

fix: trying to make progress on selection, active index, keyboard, an…

90b6c90

…d moust events

chore: aria support

41ebfa0

chore: implementing removing the click area when you click outside th…

c4ec055

…e ref

fix: fmt?

b02da6a

chore: active index logic, keyboard arrow logic

5e08013

chore: outline offset logic

01337c1

chore: some arrow key usage, need to move data up a level I think

26e0fba

chore: more data up a level

5c97b3d

fix: improving file/folder structure and resolving typescript errors

b5b1d7d

chore: clean up

bc2ba07

fix: fmt

8b9776c

fix: hmm trying to deal with state management

9338dde

fix: moving the filter logic up

d728667

chore: keyboard interactions

0b24f0a

fix: ope

e429ff7

fix:commiting broken context menu

bc58484

fix: some testing code for dragging

7ef9b25

chore:debug with react dev tools

9308ffe

nadr0 mentioned this pull request Jul 1, 2025

[feature] Review https://github.com/KittyCAD/modeling-app/pull/7656.. optimizations got out of hand within this single PR #7592

Closed

nadr0 marked this pull request as ready for review July 1, 2025 20:01

nadr0 requested a review from a team as a code owner July 1, 2025 20:01

graphite-app bot reviewed Jul 1, 2025

View reviewed changes

src/components/Explorer/ProjectExplorer.tsx Outdated Show resolved Hide resolved

vercel bot deployed to Preview July 1, 2025 20:15 View deployment

fix: ope, wrong function

5ec5759

vercel bot deployed to Preview July 1, 2025 20:29 View deployment

pierremtb mentioned this pull request Jul 8, 2025

Rename Clone file menu item to Duplicate #5288

Closed

franknoirot changed the title ~~Nadro/gh 6878/no virtual list~~ Refactor file explorer to be a pure component (no virtualization yet) Jul 9, 2025

andrewvarga reviewed Jul 9, 2025

View reviewed changes

pierremtb added this to the General App Stuff: State and Layout milestone Jul 15, 2025

fix: merging main

8122586

vercel bot deployed to Preview July 21, 2025 18:50 View deployment

nadr0 changed the title ~~Refactor file explorer to be a pure component (no virtualization yet)~~ feature: Project Explorer update, first pass. Jul 23, 2025

Merge branch 'main' into nadro/gh-6878/no-virtual-list

6e1686d

vercel bot deployed to Preview July 23, 2025 14:41 View deployment

nadr0 commented Jul 23, 2025

View reviewed changes

fix: addressing PR comments, implementing close folder if you are on …

3e55b8d

…first file and press left arrow

vercel bot deployed to Preview July 23, 2025 20:03 View deployment

		@@ -0,0 +1,175 @@
		import React, { useEffect, useMemo } from 'react'

feature: Project Explorer update, first pass. #7656

Are you sure you want to change the base?

feature: Project Explorer update, first pass. #7656

Uh oh!

Conversation

nadr0 commented Jul 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

nadr0 commented Jul 1, 2025

Uh oh!

jacebrowning commented Jul 2, 2025

Uh oh!

franknoirot commented Jul 9, 2025

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

andrewvarga commented Jul 9, 2025

Uh oh!

andrewvarga commented Jul 9, 2025

Uh oh!

nadr0 commented Jul 23, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nadr0 commented Jul 23, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nadr0 commented Jul 23, 2025

Uh oh!

Uh oh!

nadr0 commented Jul 1, 2025 •

edited

Loading

nadr0 commented Jul 23, 2025 •

edited

Loading