Skip to content

Commit

Permalink
INT-3299: Fixed id prop logging a warning when it shouldn't (#392)
Browse files Browse the repository at this point in the history
* INT-3299: Check that an existing element with an id does not equal the current elementRef

* INT-3299: Added captureLogs function

* INT-3299: Added chai package for testing as well allowing it in eslint

* INT-3299: Added PropTest.ts

* INT-3299: Removed a comment

* INT-3299: Increased timeout

* INT-3299: use a `pTryUntil` instead of `waitForEditorsToLoad`
  • Loading branch information
danoaky-tiny authored Jul 8, 2024
1 parent 926dc87 commit 6ca801c
Show file tree
Hide file tree
Showing 6 changed files with 3,825 additions and 3,493 deletions.
10 changes: 9 additions & 1 deletion .eslintrc.json
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,17 @@
{
"files": [
"**/*Test.ts",
"**/test/ts/**/*.ts"
"**/test/**/*.ts"
],
"plugins": [
"chai-friendly"
],
"extends": [
"plugin:chai-friendly/recommended"
],
"rules": {
"no-unused-expressions": "off",
"no-console": "off",
"max-classes-per-file": "off",
"@typescript-eslint/no-non-null-assertion": "off"
}
Expand Down
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -43,12 +43,15 @@
"@tinymce/beehive-flow": "^0.19.0",
"@tinymce/eslint-plugin": "^2.3.1",
"@tinymce/miniature": "^6.0.0",
"@types/chai": "^4.3.16",
"@types/node": "^20.11.30",
"autoprefixer": "^10.4.19",
"babel-loader": "^9.1.3",
"chai": "^5.1.1",
"codelyzer": "^6.0.2",
"copyfiles": "^2.4.1",
"core-js": "^3.36.1",
"eslint-plugin-chai-friendly": "^1.0.0",
"eslint-plugin-storybook": "^0.8.0",
"gh-pages": "^6.1.0",
"json": "11.0.0",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,8 @@ export class EditorComponent extends Events implements AfterViewInit, ControlVal
const tagName = typeof this.tagName === 'string' ? this.tagName : 'div';
this._element = document.createElement(this.inline ? tagName : 'textarea');
if (this._element) {
if (document.getElementById(this.id)) {
const existingElement = document.getElementById(this.id);
if (existingElement && existingElement !== this._elementRef.nativeElement) {
/* eslint no-console: ["error", { allow: ["warn"] }] */
console.warn(`TinyMCE-Angular: an element with id [${this.id}] already exists. Editors with duplicate Id will not be able to mount`);
}
Expand Down
24 changes: 19 additions & 5 deletions tinymce-angular-component/src/test/ts/alien/TestHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ import { EditorComponent } from '../../../main/ts/editor/editor.component';
import { Editor } from 'tinymce';
import { Keyboard, Keys } from '@ephox/agar';

export const apiKey = Fun.constant(
'qagffr3pkuv17a8on1afax661irst1hbr4e6tbv888sz91jc',
);
export const apiKey = Fun.constant('qagffr3pkuv17a8on1afax661irst1hbr4e6tbv888sz91jc');

export const throwTimeout =
(timeoutMs: number, message: string = `Timeout ${timeoutMs}ms`) =>
Expand All @@ -19,7 +17,7 @@ export const throwTimeout =
timeout({
first: timeoutMs,
with: () => throwError(() => new Error(message)),
}),
})
);

export const deleteTinymce = () => {
Expand All @@ -28,7 +26,8 @@ export const deleteTinymce = () => {
delete Global.tinymce;
delete Global.tinyMCE;

const hasTinyUri = (attrName: string) => (elm: SugarElement<Element>) => Attribute.getOpt(elm, attrName).exists((src) => Strings.contains(src, 'tinymce'));
const hasTinyUri = (attrName: string) => (elm: SugarElement<Element>) =>
Attribute.getOpt(elm, attrName).exists((src) => Strings.contains(src, 'tinymce'));

const elements = Arr.flatten([
Arr.filter(SelectorFilter.all('script'), hasTinyUri('src')),
Expand All @@ -38,6 +37,21 @@ export const deleteTinymce = () => {
Arr.each(elements, Remove.remove);
};

export const captureLogs = async (
method: 'log' | 'warn' | 'debug' | 'error',
fn: () => Promise<void> | void
): Promise<unknown[][]> => {
const original = console[method];
try {
const logs: unknown[][] = [];
console[method] = (...args: unknown[]) => logs.push(args);
await fn();
return logs;
} finally {
console[method] = original;
}
};

export const fakeTypeInEditor = (fixture: ComponentFixture<unknown>, str: string) => {
const editor: Editor = fixture.debugElement.query(By.directive(EditorComponent)).componentInstance.editor!;
editor.getBody().innerHTML = '<p>' + str + '</p>';
Expand Down
120 changes: 120 additions & 0 deletions tinymce-angular-component/src/test/ts/browser/PropTest.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
import '../alien/InitTestEnvironment';

import { Component, Type } from '@angular/core';
import { context, describe, it } from '@ephox/bedrock-client';

import { EditorComponent } from '../../../main/ts/public_api';
import { eachVersionContext, fixtureHook } from '../alien/TestHooks';
import { captureLogs, throwTimeout } from '../alien/TestHelpers';
import { concatMap, distinct, firstValueFrom, mergeMap, of, toArray } from 'rxjs';
import { ComponentFixture } from '@angular/core/testing';
import { By } from '@angular/platform-browser';
import { Editor } from 'tinymce';
import { expect } from 'chai';
import { Fun } from '@ephox/katamari';
import { Waiter } from '@ephox/agar';

describe('PropTest', () => {
const containsIDWarning = (logs: unknown[][]) =>
logs.length > 0 &&
logs.some((log) => {
const [ message ] = log;
return (
typeof message === 'string' &&
message.includes('TinyMCE-Angular: an element with id [') &&
message.includes('Editors with duplicate Id will not be able to mount')
);
});
const findAllComponents = <T>(fixture: ComponentFixture<unknown>, component: Type<T>): T[] =>
fixture.debugElement.queryAll(By.directive(component)).map((v) => v.componentInstance as T);
const waitForEditorsToLoad = (fixture: ComponentFixture<unknown>): Promise<Editor[]> =>
firstValueFrom(
of(
findAllComponents(fixture, EditorComponent)
.map((ed) => ed.editor)
.filter((editor): editor is Editor => !!editor)
).pipe(
mergeMap(Fun.identity),
distinct((editor) => editor.id),
concatMap((editor) => new Promise<Editor>((resolve) => editor.once('SkinLoaded', () => resolve(editor)))),
toArray(),
throwTimeout(20000, 'Timeout waiting for editor(s) to load')
)
);

eachVersionContext([ '4', '5', '6', '7' ], () => {
context('Single editor with ID', () => {
@Component({
standalone: true,
imports: [ EditorComponent ],
template: `<editor id="my-id" />`,
})
class EditorWithID {}
const createFixture = fixtureHook(EditorWithID, { imports: [ EditorWithID ] });

it('INT-3299: setting an ID does not log a warning', async () => {
const warnings = await captureLogs('warn', async () => {
const fixture = createFixture();
fixture.detectChanges();
const [ ed ] = await waitForEditorsToLoad(fixture);
expect(ed.id).to.equal('my-id');
});
expect(containsIDWarning(warnings), 'Should not contain an ID warning').to.be.false;
});
});

context('Multiple editors', () => {
@Component({
standalone: true,
imports: [ EditorComponent ],
template: `
@for (props of editors; track props) {
<editor [id]="props.id" [plugins]="props.plugins" [init]="props.init" [initialValue]="props.initialValue" />
}
`,
})
class MultipleEditors {
public editors: Partial<Pick<EditorComponent, 'id' | 'init' | 'plugins' | 'initialValue'>>[] = [];
}
const createFixture = fixtureHook(MultipleEditors, { imports: [ MultipleEditors ] });

it('INT-3299: creating more than one editor with the same ID logs a warning', async () => {
const warnings = await captureLogs('warn', async () => {
const fixture = createFixture();
fixture.componentInstance.editors = [
{ id: 'my-id-0', initialValue: 'text1' },
{ id: 'my-id-0', initialValue: 'text2' },
];
fixture.detectChanges();
const [ ed1, ed2 ] = await waitForEditorsToLoad(fixture);
expect(ed1.id).to.equal('my-id-0');
expect(ed1.getContent()).to.equal('<p>text1</p>');
expect(ed2).to.be.undefined;
});
expect(containsIDWarning(warnings), 'Should contain an ID warning').to.be.true;
});

it('INT-3299: creating more than one editor with different IDs does not log a warning', async () => {
const warnings = await captureLogs('warn', async () => {
const fixture = createFixture();
fixture.componentInstance.editors = Array.from({ length: 3 }, (_, i) => ({
id: `my-id-${i}`,
initialValue: `text${i}`,
}));
fixture.detectChanges();
const [ ed1, ed2, ed3, ed4 ] = findAllComponents(fixture, EditorComponent);
await Waiter.pTryUntil('All editors to have been initialised', () => {
expect(ed1.id).to.equal('my-id-0');
expect(ed1.editor?.getContent()).to.equal('<p>text0</p>');
expect(ed2.id).to.equal('my-id-1');
expect(ed2.editor?.getContent()).to.equal('<p>text1</p>');
expect(ed3.id).to.equal('my-id-2');
expect(ed3.editor?.getContent()).to.equal('<p>text2</p>');
expect(ed4?.editor).to.be.undefined;
}, 1000, 10000);
});
expect(containsIDWarning(warnings), 'Should not contain an ID warning').to.be.false;
});
});
});
});
Loading

0 comments on commit 6ca801c

Please sign in to comment.