-
Notifications
You must be signed in to change notification settings - Fork 12
Populate ChooseFile modal via file-meta entries in index #3949
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
Populate ChooseFile modal via file-meta entries in index #3949
Conversation
Preview deployments |
061876a to
07c14a0
Compare
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.
Pull request overview
This PR replaces HTTP directory listing requests with indexed file-meta queries for the ChooseFile modal. Instead of making separate HTTP requests for each directory level as users expand directories, the new implementation queries all files at once via the search API and builds the tree structure client-side. This approach leverages SearchResource for Store integration, caching, reference counting, and live event subscription, which should improve performance and provide real-time updates when files are added or removed.
Changes:
- Introduces
FileTreeFromIndexResourcethat queries all FileDef instances in a realm and builds a tree structure from them - Creates
IndexedFileTreecomponent that renders the file tree using the new resource - Updates
choose-file-modalto use the new IndexedFileTree component with a component recreation pattern when realms switch
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| packages/host/app/resources/file-tree-from-index.ts | New resource that wraps SearchResource with FileDef type filter, queries all files at once, and builds hierarchical tree structure from flat file list |
| packages/host/app/components/editor/indexed-file-tree.gts | New component that renders file tree using FileTreeFromIndexResource, manages directory expansion state, and provides loading indicators |
| packages/host/app/components/operator-mode/choose-file-modal.gts | Updates to use IndexedFileTree instead of FileTree, with #each pattern to force component recreation when realm changes |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| onFileSelected: (entryPath: LocalPath) => void; | ||
| onDirectorySelected: (entryPath: LocalPath) => void; | ||
| scrollPositionKey?: LocalPath; | ||
| relativePath: string; |
Copilot
AI
Feb 6, 2026
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.
The relativePath parameter is declared in the TreeLevelSignature but is never used in the TreeLevel component. It's passed down recursively but has no effect on the component's behavior. Consider removing it if it's not needed, or document why it's being tracked for future use.
|
|
||
| class TreeLevel extends Component<TreeLevelSignature> { | ||
| <template> | ||
| {{#each @entries as |entry|}} |
Copilot
AI
Feb 6, 2026
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.
The #each loop should include a key parameter for better rendering performance and correctness. The old Directory component uses key='path' which should be added here as well. Without a key, Ember may not correctly track which items have changed when the list updates, potentially causing rendering issues or unnecessary re-renders.
| {{#each @entries as |entry|}} | |
| {{#each @entries key="path" as |entry|}} |
| // We query with FileDef type filter, so instances are FileDef | ||
| let files = this.search.instances as FileDef[]; | ||
| let tree = this.buildTreeFromFiles(files); | ||
| return this.sortEntries(tree); | ||
| } | ||
|
|
Copilot
AI
Feb 6, 2026
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.
The type assertion as FileDef[] assumes all instances are FileDef without runtime validation. If the search returns unexpected types (e.g., due to index inconsistencies or race conditions), this could cause issues downstream. Consider adding runtime validation or a filter to ensure only FileDef instances are processed.
| // We query with FileDef type filter, so instances are FileDef | |
| let files = this.search.instances as FileDef[]; | |
| let tree = this.buildTreeFromFiles(files); | |
| return this.sortEntries(tree); | |
| } | |
| let instances = this.search.instances ?? []; | |
| let files = instances.filter((instance): instance is FileDef => | |
| this.isFileDef(instance), | |
| ); | |
| let tree = this.buildTreeFromFiles(files); | |
| return this.sortEntries(tree); | |
| } | |
| private isFileDef(instance: CardDef | FileDef): instance is FileDef { | |
| let anyInstance = instance as any; | |
| return ( | |
| anyInstance?.type?.module === 'https://cardstack.com/base/file-api' && | |
| anyInstance?.type?.name === 'FileDef' | |
| ); | |
| } |
0458b9c to
6da65ad
Compare
Replace HTTP directory listing requests with indexed file-meta queries for the ChooseFile modal. Instead of making separate HTTP requests for each directory level as users expand the tree, we now query all files at once via the search API and build the tree client-side. Changes: - Add FileTreeFromIndexResource that wraps SearchResource with FileDef type filter and builds a tree structure from flat file instances - Add IndexedFileTree component that renders the tree using the resource - Update ChooseFileModal to use IndexedFileTree instead of FileTree The new implementation leverages SearchResource for Store integration, caching, reference counting, and live event subscription. Closes CS-10109 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use private class field (#realmURL) instead of tracked property to avoid "attempted to update but it had already been used" error during render. Private fields are not automatically tracked by Glimmer, which prevents the conflict when modify() updates the URL after entries getter has read it. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
The comparison to skip re-creating the search was causing issues when switching realms - the search resource's realms function was evaluated with stale data. The SearchResource handles deduplication internally, so we can safely remove this optimization. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Use {{#each}} with single-element array to force component recreation
when the realm URL changes. This ensures the FileTreeFromIndexResource
is properly re-initialized with the new realm URL.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Show a centered LoadingIndicator spinner in the file tree mask overlay while the search query is still loading, so users see feedback instead of a blank white area in the ChooseFileModal. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Switch from SearchResource (hydrated instances) to SearchDataResource
(raw JSON:API resources) with asData: true and sparse fieldset
fields: { 'file-meta': [] } for minimal payload
- Skip loadLinks for asData queries in Realm.search() to avoid
FilterRefersToNonexistentTypeError on stale type references
- Fix loading spinner positioning with min-height: 100% on nav
- Sort entries alphabetically (matching original file chooser behavior)
- Decode percent-encoded URL path segments for proper display
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
6da65ad to
d642938
Compare
|
Issues reported by @backspace have been addressed except for the emoji issue, which I'll spin off into a separate PR |


Summary
Changes
packages/host/app/resources/file-tree-from-index.tspackages/host/app/components/editor/indexed-file-tree.gtspackages/host/app/components/operator-mode/choose-file-modal.gtsTest plan
Closes CS-10109
🤖 Generated with Claude Code