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

INT-3299: Fixed id prop logging a warning when it shouldn't #392

Merged
merged 8 commits into from
Jul 8, 2024
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