feat: add headless workspace comment class #7916
Conversation
BeksOmega
force-pushed
the
feat/workspace-comment
branch
2 times, most recently
from
2f65abf
to
e6ac429
Compare
BeksOmega
force-pushed
the
feat/workspace-comment
branch
from
e6ac429
to
dbac646
Compare
|
@cpcallen Since Maribeth seems to be maybe OOO would you be interested in taking this? |
| * workspace is read-only. | ||
| */ | ||
| isEditable(): boolean { | ||
| return this.isOwnEditable() && !this.workspace.options.readOnly; |
There was a problem hiding this comment.
swapping these two is slightly better in case the workspace is readonly and the user decides to compute the traveling salesman problem in the ownEditable function, but i'll acknowledge this is minute :P
the equivalent in Block also checks if the block has been disposed. do we need to do that here?
There was a problem hiding this comment.
I don't think we need to do that. Looks like in Block it was originally (this.workspace && !this.workspace.options.readonly) which turned into !this.disposed && !this.workspace.options.readonly which turned into !this.isDeadOrDying() && !this.workspace.options.readonly.
So I think it was originally just a type safety thing, and as we migrated code the logic got slightly confused :P
Theoretically something that is being disposed probably isn't editable? But it probably doesn't have a size or text either. I think it's fine to ignore this.
core/comments/workspace_comment.ts
Outdated
| protected readonly workspace: Workspace, | ||
| id?: string, | ||
| ) { | ||
| // TODO: Before merging, file issue to update getCommentById. |
There was a problem hiding this comment.
you have some todos here fyi
The basics
The details
Resolves
Fixes #7906
Proposed Changes + reasons
Adds a headless workspace comment class (just holds data, doesn't refer to any view elements) to support headless mode.
Test Coverage
No testing, because this is just a bunch of data storage methods. These methods will be covered by later serialization tests.
Documentation
N/A
Additional Information
N/A