Skip to content
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

Sync dirty property between clients #11525

Merged
merged 3 commits into from Nov 30, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 1 addition & 2 deletions packages/docregistry/src/context.ts
Expand Up @@ -676,7 +676,6 @@ export class Context<
if (this.isDisposed) {
return;
}
const dirty = false;
if (contents.format === 'json') {
model.fromJSON(contents.content);
if (initializeModel) {
Expand All @@ -701,7 +700,7 @@ export class Context<
}
}
this._updateContentsModel(contents);
model.dirty = dirty;
model.dirty = false;
if (!this._isPopulated) {
return this._populate();
}
Expand Down
24 changes: 18 additions & 6 deletions packages/docregistry/src/default.ts
Expand Up @@ -29,6 +29,9 @@ export class DocumentModel
const filemodel = new models.YFile() as models.ISharedFile;
this.switchSharedModel(filemodel, true);
this.value.changed.connect(this.triggerContentChange, this);

(this.sharedModel as models.YFile).dirty = false;
Copy link
Member

Choose a reason for hiding this comment

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

@hbcarlos We should avoid parsing to the concrete Y* implementations because that is non-ideomatic.

In general, when you work with an abstraction, you shouldn't cast to the concrete implementation just to manipulate properties of the concrete implementation because that will eventually lead to issues.

In this case, we explicitly don't want to initialize shared properties whenever a user joins a session. Properties on the concrete Yjs implementation must only be initialized on the create and createStandalone methods.

this.sharedModel.changed.connect(this._onStateChanged, this);
}

/**
Expand All @@ -49,15 +52,14 @@ export class DocumentModel
* The dirty state of the document.
*/
get dirty(): boolean {
return this._dirty;
return this.sharedModel.dirty;
}
set dirty(newValue: boolean) {
if (newValue === this._dirty) {
const oldValue = this.sharedModel.dirty;
if (newValue === oldValue) {
return;
}
const oldValue = this._dirty;
this._dirty = newValue;
this.triggerStateChange({ name: 'dirty', oldValue, newValue });
(this.sharedModel as models.YFile).dirty = newValue;
}

/**
Expand Down Expand Up @@ -151,12 +153,22 @@ export class DocumentModel
this.dirty = true;
}

private _onStateChanged(
sender: models.ISharedFile,
changes: models.NotebookChange
): void {
if (changes.stateChange) {
changes.stateChange.forEach(value => {
this.triggerStateChange(value);
});
}
}

/**
* The shared notebook model.
*/
readonly sharedModel: models.ISharedFile;
private _defaultLang = '';
private _dirty = false;
private _readOnly = false;
private _contentChanged = new Signal<this, void>(this);
private _stateChanged = new Signal<this, IChangedArgs<any>>(this);
Expand Down
14 changes: 5 additions & 9 deletions packages/notebook/src/model.ts
Expand Up @@ -114,6 +114,7 @@ export class NotebookModel implements INotebookModel {
metadata.changed.connect(this._onMetadataChanged, this);
this._deletedCells = [];

(this.sharedModel as models.YNotebook).dirty = false;
this.sharedModel.changed.connect(this._onStateChanged, this);
}
/**
Expand All @@ -134,15 +135,14 @@ export class NotebookModel implements INotebookModel {
* The dirty state of the document.
*/
get dirty(): boolean {
return this._dirty;
return this.sharedModel.dirty;
}
set dirty(newValue: boolean) {
if (newValue === this._dirty) {
const oldValue = this.sharedModel.dirty;
if (newValue === oldValue) {
return;
}
const oldValue = this._dirty;
this._dirty = newValue;
this.triggerStateChange({ name: 'dirty', oldValue, newValue });
(this.sharedModel as models.YNotebook).dirty = newValue;
}

/**
Expand Down Expand Up @@ -428,9 +428,6 @@ close the notebook without saving it.`,
if (value.name === 'nbformatMinor') {
this._nbformatMinor = value.newValue;
}
if (value.name === 'dirty') {
this._dirty = value.newValue;
}
this.triggerStateChange(value);
});
}
Expand Down Expand Up @@ -513,7 +510,6 @@ close the notebook without saving it.`,
*/
readonly modelDB: IModelDB;

private _dirty = false;
private _readOnly = false;
private _contentChanged = new Signal<this, void>(this);
private _stateChanged = new Signal<this, IChangedArgs<any>>(this);
Expand Down
5 changes: 5 additions & 0 deletions packages/shared-models/src/api.ts
Expand Up @@ -58,6 +58,11 @@ export interface ISharedBase extends IDisposable {
* This is used by, for example, docregistry to share the file-path of the edited content.
*/
export interface ISharedDocument extends ISharedBase {
/**
* Whether the document is saved to disk or not.
*/
readonly dirty: boolean;

/**
* The changed signal.
*/
Expand Down
22 changes: 17 additions & 5 deletions packages/shared-models/src/ymodels.ts
Expand Up @@ -26,6 +26,16 @@ export interface IYText extends models.ISharedText {
export type YCellType = YRawCell | YCodeCell | YMarkdownCell;

export class YDocument<T> implements models.ISharedDocument {
get dirty(): boolean {
return this.ystate.get('dirty');
}

set dirty(value: boolean) {
this.transact(() => {
this.ystate.set('dirty', value);
}, false);
}

/**
* Perform a transaction. While the function f is called, all changes to the shared
* document are bundled into a single event.
Expand Down Expand Up @@ -128,11 +138,13 @@ export class YFile

event.keysChanged.forEach(key => {
const change = event.changes.keys.get(key);
stateChange.push({
name: key,
oldValue: change?.oldValue ? change!.oldValue : 0,
newValue: this.ystate.get(key)
});
if (change) {
stateChange.push({
name: key,
oldValue: change.oldValue,
newValue: this.ystate.get(key)
});
}
});

this._changed.emit({ stateChange });
Expand Down