Skip to content

Commit

Permalink
Merge pull request #126 from ckeditor/t/121-b
Browse files Browse the repository at this point in the history
Improve stability of package and tests
  • Loading branch information
Dumluregn authored Aug 7, 2020
2 parents 4c94a77 + a43fff7 commit ebf6aed
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 91 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
Other Changes:

* [#98](https://github.com/ckeditor/ckeditor4-angular/issues/98): Updated repository dependencies (no changes in the actual `ckeditor4-angular` package).
* [#128](https://github.com/ckeditor/ckeditor4-angular/issues/128): Improve the stability of `getEditorNamespace()` method.

## ckeditor4-angular 1.2.2

Expand Down
2 changes: 1 addition & 1 deletion src/app/demo-form/demo-form.component.css
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ form > div {

pre {
word-wrap: break-word;
white-space: pre-wrap;
white-space: pre-wrap;
}

p.alert {
Expand Down
2 changes: 1 addition & 1 deletion src/app/demo-form/demo-form.component.html
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ <h3>User profile form</h3>
[(ngModel)]="model.description"
id="description"
name="description"
type="divarea">
type="divarea">
</ckeditor>

<p *ngIf="description && description.dirty" class="alert">Description is "dirty".</p>
Expand Down
100 changes: 44 additions & 56 deletions src/ckeditor/ckeditor.component.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,11 @@
*/

import waitUntil from 'wait-until-promise';
import { async, ComponentFixture, TestBed } from '@angular/core/testing';
import { ComponentFixture, TestBed } from '@angular/core/testing';
import { CKEditorComponent } from './ckeditor.component';
import {
fireDragEvent,
mockDropEvent,
mockPasteEvent,
setDataMultipleTimes,
whenDataReady,
whenEvent
Expand All @@ -24,11 +23,11 @@ describe( 'CKEditorComponent', () => {
fixture: ComponentFixture<CKEditorComponent>,
config: Object;

beforeEach( async( () => {
TestBed.configureTestingModule( {
beforeEach( () => {
return TestBed.configureTestingModule( {
declarations: [ CKEditorComponent ]
} ).compileComponents();
} ) );
} )

beforeEach( () => {
fixture = TestBed.createComponent( CKEditorComponent );
Expand Down Expand Up @@ -73,9 +72,10 @@ describe( 'CKEditorComponent', () => {
} );

it( 'should have proper editor type', () => {
whenEvent( 'ready', component ).then( () => {
return whenEvent( 'ready', component ).then( () => {
fixture.detectChanges();
expect( component.instance.editable().isInline() ).toBe( component.type !== EditorType.CLASSIC );
expect( component.instance.editable().isInline() )
.toBe( component.type !== EditorType.CLASSIC );
} );
} );

Expand Down Expand Up @@ -141,7 +141,7 @@ describe( 'CKEditorComponent', () => {
warn: isDivarea
} ].forEach( ( { newConfig, msg, warn } ) => {
describe( msg, () => {
beforeAll( () => {
beforeEach( () => {
config = newConfig;
} );

Expand Down Expand Up @@ -170,14 +170,15 @@ describe( 'CKEditorComponent', () => {
} );

describe( 'when set with config', () => {
beforeEach( done => {
beforeEach( () => {
component.config = {
readOnly: true,
width: 1000,
height: 1000
};
fixture.detectChanges();
whenEvent( 'ready', component ).then( done );

return whenEvent( 'ready', component );
} );

it( 'editor should be readOnly', () => {
Expand All @@ -193,84 +194,80 @@ describe( 'CKEditorComponent', () => {
expect( component.instance.plugins.undo ).not.toBeUndefined();
} );

it( 'should register changes', async done => {
it( 'should register changes', () => {
const spy = jasmine.createSpy();

component.registerOnChange( spy );

setDataMultipleTimes( component.instance, [
return setDataMultipleTimes( component.instance, [
'<p>Hello World!</p>',
'<p>I am CKEditor for Angular!</p>'
] ).then( () => {
expect( spy ).toHaveBeenCalledTimes( 2 );

done();
} );
} );
} );

describe( 'when set without undo plugin', () => {
beforeEach( ( done ) => {
beforeEach( () => {
component.config = {
removePlugins: 'undo'
};
fixture.detectChanges();
whenEvent( 'ready', component ).then( done );
return whenEvent( 'ready', component );
} );

it( 'editor should not have undo plugin', () => {
expect( component.instance.plugins.undo ).toBeUndefined();
} );

it( 'should register changes without undo plugin', async done => {
it( 'should register changes without undo plugin', () => {
const spy = jasmine.createSpy();

component.registerOnChange( spy );

setDataMultipleTimes( component.instance, [
return setDataMultipleTimes( component.instance, [
'<p>Hello World!</p>',
'<p>I am CKEditor for Angular!</p>'
] ).then( () => {
expect( spy ).toHaveBeenCalledTimes( 2 );

done();
} );
} );
} );
} );

describe( 'on destroy', () => {
it ( 'should not have call runOutsideAngular when destroy before DOM loaded', done => {
it ( 'should not have call runOutsideAngular when destroy before DOM loaded', () => {
spyOn( fixture.ngZone, 'runOutsideAngular' );

fixture.detectChanges();

waitUntil( () => {
return waitUntil( () => {
fixture.destroy();
return true;
}, 0 ).then( () => {
expect( fixture.ngZone.runOutsideAngular ).toHaveBeenCalledTimes( 1 );
} ).then( done );
} );
} );

it ( 'should not have call runOutsideAngular when destroy before DOM loaded', done => {
it ( 'should not have call runOutsideAngular when destroy before DOM loaded', () => {
spyOn( fixture.ngZone, 'runOutsideAngular' );

fixture.detectChanges();

waitUntil( () => {
return waitUntil( () => {
fixture.destroy();
return true;
}, 200 ).then( () => {
expect( fixture.ngZone.runOutsideAngular ).toHaveBeenCalledTimes( 1 );
} ).then( done );
} );
} );
} );

describe( 'when component is ready', () => {
beforeEach( done => {
beforeEach( () => {
fixture.detectChanges();
whenEvent( 'ready', component ).then( done );
return whenEvent( 'ready', component );
} );

it( 'should be initialized', () => {
Expand Down Expand Up @@ -319,18 +316,16 @@ describe( 'CKEditorComponent', () => {
const data = '<b>foo</b>',
expected = '<p><strong>foo</strong></p>\n';

it( 'should be configurable at the start of the component', async done => {
it( 'should be configurable at the start of the component', async () => {
fixture.detectChanges();

await whenDataReady( component.instance, () => component.data = data );

expect( component.data ).toEqual( expected );
expect( component.instance.getData() ).toEqual( expected );

done();
} );

it( 'should be writeable by ControlValueAccessor', async done => {
it( 'should be writeable by ControlValueAccessor', async () => {
fixture.detectChanges();

const editor = component.instance;
Expand All @@ -342,8 +337,6 @@ describe( 'CKEditorComponent', () => {
await whenDataReady( editor, () => component.writeValue( '<p><i>baz</i></p>' ) );

expect( component.instance.getData() ).toEqual( '<p><em>baz</em></p>\n' );

done();
} );
} );

Expand Down Expand Up @@ -390,7 +383,7 @@ describe( 'CKEditorComponent', () => {
const editable = component.instance.editable();
const editor = editable.getEditor( false );

const eventPromise = whenEvent( 'paste', component ).then( () => {
const eventPromise = whenEvent( 'paste', component ).then( () => {
expect( spy ).toHaveBeenCalledTimes( 1 );
expect( component.instance.getData() ).toEqual( '<p>bam</p>\n' );
} );
Expand Down Expand Up @@ -423,7 +416,7 @@ describe( 'CKEditorComponent', () => {
return eventPromise;
} );

it( 'drag/drop events should emit component dragStart, dragEnd and drop', async done => {
it( 'drag/drop events should emit component dragStart, dragEnd and drop', () => {
fixture.detectChanges();

const spyDragStart = jasmine.createSpy( 'dragstart' );
Expand All @@ -435,30 +428,26 @@ describe( 'CKEditorComponent', () => {
const spyDrop = jasmine.createSpy( 'drop' );
component.drop.subscribe( spyDrop );

whenDataReady( component.instance, () => {
const dropEvent = mockDropEvent();
const paragraph = component.instance.editable().findOne( 'p' );
const dropEvent = mockDropEvent();
const paragraph = component.instance.editable().findOne( 'p' );

component.instance.getSelection().selectElement( paragraph );
component.instance.getSelection().selectElement( paragraph );

fireDragEvent( 'dragstart', component.instance, dropEvent );
fireDragEvent( 'dragstart', component.instance, dropEvent );

expect( spyDragStart ).toHaveBeenCalledTimes( 1 );
expect( spyDragStart ).toHaveBeenCalledTimes( 1 );

fireDragEvent( 'dragend', component.instance, dropEvent );
fireDragEvent( 'dragend', component.instance, dropEvent );

expect( spyDragEnd ).toHaveBeenCalledTimes( 1 );
expect( spyDragEnd ).toHaveBeenCalledTimes( 1 );

// There is some issue in Firefox with simulating drag-drop flow. The drop event
// is not fired making this assertion fail. Let's skip it for now.
if ( !CKEDITOR.env.gecko ) {
fireDragEvent( 'drop', component.instance, dropEvent );
// There is some issue in Firefox with simulating drag-drop flow. The drop event
// is not fired making this assertion fail. Let's skip it for now.
if ( !CKEDITOR.env.gecko ) {
fireDragEvent( 'drop', component.instance, dropEvent );

expect( spyDrop ).toHaveBeenCalledTimes( 1 );
}

done();
} );
expect( spyDrop ).toHaveBeenCalledTimes( 1 );
}
} );

it( 'fileUploadRequest should emit component fileUploadRequest', () => {
Expand Down Expand Up @@ -517,17 +506,16 @@ describe( 'CKEditorComponent', () => {
expect( spy ).toHaveBeenCalled();
} );

it( 'onChange callback should be called when editor model changes', async done => {
it( 'onChange callback should be called when editor model changes', () => {
fixture.detectChanges();

const spy = jasmine.createSpy();
component.registerOnChange( spy );

setDataMultipleTimes( component.instance, [
return setDataMultipleTimes( component.instance, [
'initial', 'initial', 'modified'
] ).then( () => {
expect( spy ).toHaveBeenCalledTimes( 2 );
done();
} );
} );
} );
Expand Down
26 changes: 14 additions & 12 deletions src/ckeditor/ckeditor.helpers.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ declare let CKEDITOR: any;

describe( 'getEditorNamespace', () => {
const fakeScript = 'data:text/javascript;base64,d2luZG93LkNLRURJVE9SID0ge307';
let isIe10 = navigator.userAgent.toLowerCase().indexOf( 'trident/' ) > -1;
isIe10 = isIe10 && document[ 'documentMode' ] === 10;

beforeEach( () => {
delete window[ 'CKEDITOR' ];
Expand All @@ -21,9 +19,10 @@ describe( 'getEditorNamespace', () => {
} );

it( 'typeError thrown when empty string passed', () => {
expect( () => {
getEditorNamespace( '' );
} ).toThrowError( 'CKEditor URL must be a non-empty string.' );
return getEditorNamespace( '' ).catch( err => {
expect( err instanceof Error );
expect( err.message ).toEqual( 'CKEditor URL must be a non-empty string.' );
} );
} );

it( 'returns promise even if namespace is present', () => {
Expand All @@ -37,13 +36,11 @@ describe( 'getEditorNamespace', () => {
} );
} );

if ( !isIe10 ) { // Ignore unstable test on IE10.
it( 'load script and resolves with loaded namespace', () => {
return getEditorNamespace( fakeScript ).then( namespace => {
expect( namespace ).toBe( CKEDITOR );
} );
it( 'load script and resolves with loaded namespace', () => {
return getEditorNamespace( fakeScript ).then( namespace => {
expect( namespace ).toBe( CKEDITOR );
} );
}
} );

it( 'rejects with error when script cannot be loaded', () => {
return getEditorNamespace( 'non-existent.js' ).catch( err => {
Expand All @@ -52,6 +49,11 @@ describe( 'getEditorNamespace', () => {
} );

it( 'returns the same promise', () => {
expect( getEditorNamespace( fakeScript ) ).toBe( getEditorNamespace( fakeScript ) );
const promise1 = getEditorNamespace( fakeScript );
const promise2 = getEditorNamespace( fakeScript );

expect( promise1 ).toBe( promise2 );

return Promise.all( [ promise1, promise2 ] )
} );
} );
5 changes: 3 additions & 2 deletions src/ckeditor/ckeditor.helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ let promise;

export function getEditorNamespace( editorURL: string ): Promise<{ [ key: string ]: any; }> {
if ( editorURL.length < 1 ) {
throw new TypeError( 'CKEditor URL must be a non-empty string.' );
return Promise.reject( new TypeError( 'CKEditor URL must be a non-empty string.' ) );
}

if ( 'CKEDITOR' in window ) {
Expand All @@ -22,8 +22,9 @@ export function getEditorNamespace( editorURL: string ): Promise<{ [ key: string
scriptReject( err );
} else {
scriptResolve( CKEDITOR );
promise = undefined;
}

promise = undefined;
} );
} );
}
Expand Down
Loading

0 comments on commit ebf6aed

Please sign in to comment.