Skip to content

Commit

Permalink
fix(on-change): onchange callback wont be fired in readonly (#2773)
Browse files Browse the repository at this point in the history
* fix(on-change): onchange callback wont be fired in readonly

* do not rerender blocks on initial call
  • Loading branch information
neSpecc committed Jul 10, 2024
1 parent ba8fa73 commit 91959bb
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 8 deletions.
4 changes: 4 additions & 0 deletions docs/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# Changelog

### 2.30.2

- `Fix` – The onChange callback won't be fired when editor is initialized in the Read-Only mode

### 2.30.1

- `Fix` – Remove fake selection after multiple "convert to" inline tool toggles
Expand Down
6 changes: 6 additions & 0 deletions src/components/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,3 +3,9 @@
* {@link modules/ui.ts}
*/
export const selectionChangeDebounceTimeout = 180;

/**
* Timeout for batching of DOM changes used by the ModificationObserver
* {@link modules/modificationsObserver.ts}
*/
export const modificationsObserverBatchTimeout = 400;
3 changes: 2 additions & 1 deletion src/components/modules/modificationsObserver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { BlockId } from '../../../types';
import { BlockMutationEvent, BlockMutationType } from '../../../types/events/block';
import { ModuleConfig } from '../../types-internal/module-config';
import Module from '../__module';
import { modificationsObserverBatchTimeout } from '../constants';
import { BlockChanged, FakeCursorAboutToBeToggled, FakeCursorHaveBeenSet, RedactorDomChanged } from '../events';
import * as _ from '../utils';

Expand Down Expand Up @@ -39,7 +40,7 @@ export default class ModificationsObserver extends Module {
/**
* Fired onChange events will be batched by this time
*/
private readonly batchTime = 400;
private readonly batchTime = modificationsObserverBatchTimeout;

/**
* Prepare the module
Expand Down
21 changes: 18 additions & 3 deletions src/components/modules/readonly.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,16 +52,17 @@ export default class ReadOnly extends Module {
this.throwCriticalError();
}

this.toggle(this.config.readOnly);
this.toggle(this.config.readOnly, true);
}

/**
* Set read-only mode or toggle current state
* Call all Modules `toggleReadOnly` method and re-render Editor
*
* @param {boolean} state - (optional) read-only state or toggle
* @param state - (optional) read-only state or toggle
* @param isInitial - (optional) true when editor is initializing
*/
public async toggle(state = !this.readOnlyEnabled): Promise<boolean> {
public async toggle(state = !this.readOnlyEnabled, isInitial = false): Promise<boolean> {
if (state && this.toolsDontSupportReadOnly.length > 0) {
this.throwCriticalError();
}
Expand Down Expand Up @@ -91,6 +92,18 @@ export default class ReadOnly extends Module {
return this.readOnlyEnabled;
}

/**
* Do not re-render blocks if it's initial call
*/
if (isInitial) {
return this.readOnlyEnabled;
}

/**
* Mutex for modifications observer to prevent onChange call when read-only mode is enabled
*/
this.Editor.ModificationsObserver.disable();

/**
* Save current Editor Blocks and render again
*/
Expand All @@ -99,6 +112,8 @@ export default class ReadOnly extends Module {
await this.Editor.BlockManager.clear();
await this.Editor.Renderer.render(savedBlocks.blocks);

this.Editor.ModificationsObserver.enable();

return this.readOnlyEnabled;
}

Expand Down
67 changes: 63 additions & 4 deletions test/cypress/tests/onchange.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { BlockChangedMutationType } from '../../../types/events/block/BlockChang
import { BlockRemovedMutationType } from '../../../types/events/block/BlockRemoved';
import { BlockMovedMutationType } from '../../../types/events/block/BlockMoved';
import type EditorJS from '../../../types/index';
import { modificationsObserverBatchTimeout } from '../../../src/components/constants';

/**
* EditorJS API is passed as the first parameter of the onChange callback
Expand Down Expand Up @@ -455,7 +456,7 @@ describe('onChange callback', () => {
.get('div.ce-block')
.click();

cy.wait(500).then(() => {
cy.wait(modificationsObserverBatchTimeout).then(() => {
cy.get('@onChange').should('have.callCount', 0);
});
});
Expand Down Expand Up @@ -540,7 +541,7 @@ describe('onChange callback', () => {
/**
* Check that onChange callback was not called
*/
cy.wait(500).then(() => {
cy.wait(modificationsObserverBatchTimeout).then(() => {
cy.get('@onChange').should('have.callCount', 0);
});
});
Expand Down Expand Up @@ -607,7 +608,7 @@ describe('onChange callback', () => {
/**
* Check that onChange callback was not called
*/
cy.wait(500).then(() => {
cy.wait(modificationsObserverBatchTimeout).then(() => {
cy.get('@onChange').should('have.callCount', 0);
});
});
Expand Down Expand Up @@ -678,7 +679,7 @@ describe('onChange callback', () => {
/**
* Check that onChange callback was not called
*/
cy.wait(500).then(() => {
cy.wait(modificationsObserverBatchTimeout).then(() => {
cy.get('@onChange').should('have.callCount', 0);
});
});
Expand Down Expand Up @@ -747,6 +748,8 @@ describe('onChange callback', () => {
}));
});

cy.wait(modificationsObserverBatchTimeout);

cy.get('@onChange').should('have.callCount', 0);
});

Expand Down Expand Up @@ -845,4 +848,60 @@ describe('onChange callback', () => {
},
}));
});

it('should not be called when editor is initialized with readOnly mode', () => {
const config = {
readOnly: true,
onChange: (api, event): void => {
console.log('something changed', event);
},
data: {
blocks: [
{
type: 'paragraph',
data: {
text: 'The first paragraph',
},
},
],
},
};

cy.spy(config, 'onChange').as('onChange');

cy.createEditor(config);

cy.wait(modificationsObserverBatchTimeout);

cy.get('@onChange').should('have.callCount', 0);
});

it('should not be called when editor is switched to/from readOnly mode', () => {
createEditor([
{
type: 'paragraph',
data: {
text: 'The first paragraph',
},
},
]);

cy.get<EditorJS>('@editorInstance')
.then(async editor => {
editor.readOnly.toggle(true);
});

cy.wait(modificationsObserverBatchTimeout);

cy.get('@onChange').should('have.callCount', 0);

cy.get<EditorJS>('@editorInstance')
.then(async editor => {
editor.readOnly.toggle(false);
});

cy.wait(modificationsObserverBatchTimeout);

cy.get('@onChange').should('have.callCount', 0);
});
});

0 comments on commit 91959bb

Please sign in to comment.