Should not be able to edit file def instance in code mode#4025
Should not be able to edit file def instance in code mode#4025
Conversation
Preview deployments |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 546400fad0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| get isReadOnly() { | ||
| return !this.realm.canWrite(this.readyFile.url); | ||
| return !this.realm.canWrite(this.readyFile.url) || this.isFileDefInstance; |
There was a problem hiding this comment.
For writable realms, isReadOnly remains false until the async store.get(fileUrl, { type: 'file-meta' }) call finishes and sets fileDefResource.value, so a file-def instance can briefly open as editable; on slower responses, users can type (and potentially trigger autosave) before the lock engages. This undermines the new guarantee that file-def instances are not editable in code mode.
Useful? React with 👍 / 👎.
| return state; | ||
| }); | ||
|
|
||
| private get isFileDefInstance() { |
There was a problem hiding this comment.
What about this existing function? Can it be used here?
boxel/packages/runtime-common/code-ref.ts
Lines 123 to 124 in 8d45ff7
There was a problem hiding this comment.
Pull request overview
This PR makes file definition instances read-only in code mode by adding a check to determine if an opened file is a FileDef instance. Previously, files were only marked as read-only based on user write permissions and binary file status.
Changes:
- Added resource to asynchronously load file metadata and check if file is a FileDef instance
- Modified
isReadOnlygetter to include FileDef instance check - Added test to verify FileDef instances are read-only in code mode
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/host/app/components/operator-mode/code-submode.gts | Added fileDefResource to load file metadata, isFileDefInstance getter, and updated isReadOnly to check for FileDef instances |
| packages/host/tests/acceptance/code-submode/file-def-navigation-test.gts | Added test verifying that FileDef instances show read-only indicators and hide edit format options |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| get isReadOnly() { | ||
| return !this.realm.canWrite(this.readyFile.url); | ||
| return !this.realm.canWrite(this.readyFile.url) || this.isFileDefInstance; |
There was a problem hiding this comment.
The isReadOnly getter may briefly return false while the fileDefResource is still loading, potentially allowing users to start editing before the file is marked as read-only. Consider also checking the loading state to ensure the file is read-only during the resource fetch. For example: return !this.realm.canWrite(this.readyFile.url) || this.isFileDefInstance || this.fileDefResource?.isLoading;
| return !this.realm.canWrite(this.readyFile.url) || this.isFileDefInstance; | |
| return ( | |
| !this.realm.canWrite(this.readyFile.url) || | |
| this.isFileDefInstance || | |
| this.fileDefResource?.isLoading | |
| ); |
2 participants
Previously, a file was set to read-only in code mode only if the user lacked write permissions and the file was binary. In this PR, I added a check for whether the file is a
fileDefinstance, if it is, the file is now marked as read-only.