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.

Sign up for GitHub

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

Delay TinyMCE initialisation to focus #10723

Merged
merged 8 commits into from
Oct 30, 2018
Merged

Delay TinyMCE initialisation to focus #10723

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5e136af
Delay TinyMCE init to focus
ellatrix Oct 18, 2018
455b5b1
Try @aduth's hack
ellatrix Oct 18, 2018
1166576
don't verify html
ellatrix Oct 18, 2018
5e6566b
Generate new snapshots to fix unit tests
youknowriad Oct 29, 2018
bb9bed0
Fix quote block transforms
youknowriad Oct 29, 2018
b5a36c9
Add initialised class for e2e tests
ellatrix Oct 30, 2018
4c1ecdb
Try adjusting waitForRichTextInitialization
ellatrix Oct 30, 2018
4cc3a13
Try fixing the typing quickly e2e tests
youknowriad Oct 30, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion packages/block-library/src/button/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ exports[`core/button block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
gziolo marked this conversation as resolved.
Show resolved Hide resolved
data-mce-bogus="1"
/>
</div>
<div
class="editor-rich-text__tinymce wp-block-button__link"
>
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/heading/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ exports[`core/heading block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</h2>
<h2
class="editor-rich-text__tinymce"
>
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/list/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ exports[`core/list block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</ul>
<ul
class="editor-rich-text__tinymce"
>
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/paragraph/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,11 @@ exports[`core/paragraph block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</p>
<p
class="editor-rich-text__tinymce wp-block-paragraph"
>
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/preformatted/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ exports[`core/preformatted block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</pre>
<pre
class="editor-rich-text__tinymce"
>
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/pullquote/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,11 @@ exports[`core/pullquote block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</div>
<div
class="editor-rich-text__tinymce"
>
Expand Down
5 changes: 2 additions & 3 deletions packages/block-library/src/quote/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ export const settings = {
blocks: [ 'core/paragraph' ],
transform: ( { value, citation } ) => {
const paragraphs = [];
if ( value ) {
if ( value && value !== '<p></p>' ) {
Copy link
Member

Choose a reason for hiding this comment

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

I recall a discussion about this being non-ideal. Is this a "gotta get worse before getting better" thing? Do we have an issue to track the improvements?

Copy link
Member Author

Choose a reason for hiding this comment

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

See #10763.

Copy link
Member Author

Choose a reason for hiding this comment

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

@youknowriad After the enter "fix", do we still need these changes?

Copy link
Contributor

@youknowriad youknowriad Oct 30, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

yes, I think we do, that's something else I think. In this case we don't even focus the editor so the value is not changed.

paragraphs.push(
...split( create( { html: value, multilineTag: 'p' } ), '\u2028' )
.map( ( piece ) =>
Expand All @@ -125,7 +125,7 @@ export const settings = {
)
);
}
if ( citation ) {
if ( citation && citation !== '<p></p>' ) {
paragraphs.push(
createBlock( 'core/paragraph', {
content: citation,
Expand Down Expand Up @@ -189,7 +189,6 @@ export const settings = {

edit( { attributes, setAttributes, isSelected, mergeBlocks, onReplace, className } ) {
const { align, value, citation } = attributes;

return (
<Fragment>
<BlockControls>
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/quote/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,11 @@ exports[`core/quote block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</div>
<div
class="editor-rich-text__tinymce"
>
Expand Down
12 changes: 10 additions & 2 deletions packages/block-library/src/text-columns/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,11 @@ exports[`core/text-columns block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</p>
<p
class="editor-rich-text__tinymce"
>
Expand Down Expand Up @@ -55,7 +59,11 @@ exports[`core/text-columns block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</p>
<p
class="editor-rich-text__tinymce"
>
Expand Down
6 changes: 5 additions & 1 deletion packages/block-library/src/verse/test/__snapshots__/index.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,11 @@ exports[`core/verse block edit matches snapshot 1`] = `
contenteditable="true"
data-is-placeholder-visible="true"
role="textbox"
/>
>
<br
data-mce-bogus="1"
/>
</pre>
<pre
class="editor-rich-text__tinymce"
>
Expand Down
28 changes: 27 additions & 1 deletion packages/editor/src/components/rich-text/tinymce.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,9 +101,10 @@ export default class TinyMCE extends Component {
constructor() {
super();
this.bindEditorNode = this.bindEditorNode.bind( this );
this.onFocus = this.onFocus.bind( this );
}

componentDidMount() {
onFocus() {
this.initialize();
}

Expand Down Expand Up @@ -158,6 +159,11 @@ export default class TinyMCE extends Component {
browser_spellcheck: true,
entity_encoding: 'raw',
convert_urls: false,
// Disables TinyMCE's parsing to verify HTML. It makes
// initialisation a bit faster. Since we're setting raw HTML
// already with dangerouslySetInnerHTML, we don't need this to be
// verified.
verify_html: false,
ellatrix marked this conversation as resolved.
Show resolved Hide resolved
inline_boundaries_selector: 'a[href],code,b,i,strong,em,del,ins,sup,sub',
plugins: [],
} );
Expand All @@ -169,6 +175,18 @@ export default class TinyMCE extends Component {
this.editor = editor;
this.props.onSetup( editor );

// TinyMCE resets the element content on initialization, even
// when it's already identical to what exists currently. This
// behavior clobbers a selection which exists at the time of
// initialization, thus breaking writing flow navigation. The
// hack here neutralizes setHTML during initialization.
let setHTML;

editor.on( 'preinit', () => {
setHTML = editor.dom.setHTML;
editor.dom.setHTML = () => {};
} );

editor.on( 'init', () => {
// See https://github.com/tinymce/tinymce/blob/master/src/core/main/ts/keyboard/FormatShortcuts.ts
[ 'b', 'i', 'u' ].forEach( ( character ) => {
Expand All @@ -177,6 +195,8 @@ export default class TinyMCE extends Component {
[ 1, 2, 3, 4, 5, 6, 7, 8, 9 ].forEach( ( number ) => {
editor.shortcuts.remove( `access+${ number }` );
} );

editor.dom.setHTML = setHTML;
} );
},
} );
Expand Down Expand Up @@ -247,6 +267,11 @@ export default class TinyMCE extends Component {
} );
}

if ( initialHTML === '' ) {
// Ensure the field is ready to receive focus by TinyMCE.
initialHTML = '<br data-mce-bogus="1">';
ellatrix marked this conversation as resolved.
Show resolved Hide resolved
gziolo marked this conversation as resolved.
Show resolved Hide resolved
}

return createElement( tagName, {
...ariaProps,
className: classnames( className, 'editor-rich-text__tinymce' ),
Expand All @@ -258,6 +283,7 @@ export default class TinyMCE extends Component {
dangerouslySetInnerHTML: { __html: initialHTML },
onPaste,
onInput,
onFocus: this.onFocus,
} );
}
}
4 changes: 4 additions & 0 deletions test/e2e/specs/block-deletion.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
newPost,
pressWithModifier,
ACCESS_MODIFIER_KEYS,
waitForRichTextInitialization,
} from '../support/utils';

const addThreeParagraphsToNewPost = async () => {
Expand All @@ -16,8 +17,10 @@ const addThreeParagraphsToNewPost = async () => {
await clickBlockAppender();
await page.keyboard.type( 'First paragraph' );
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();
await page.keyboard.type( 'Second paragraph' );
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();
};

const clickOnBlockSettingsMenuItem = async ( buttonLabel ) => {
Expand Down Expand Up @@ -96,6 +99,7 @@ describe( 'block deletion -', () => {
// Add a third paragraph for this test.
await page.keyboard.type( 'Third paragraph' );
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();

// Press the up arrow once to select the third and fourth blocks.
await pressWithModifier( 'Shift', 'ArrowUp' );
Expand Down
6 changes: 5 additions & 1 deletion test/e2e/specs/blocks/__snapshots__/quote.test.js.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,11 @@ exports[`Quote can be converted to paragraphs and renders a paragraph for the ci
<!-- /wp:paragraph -->"
`;

exports[`Quote can be converted to paragraphs and renders a void paragraph if both the cite and quote are void 1`] = `""`;
exports[`Quote can be converted to paragraphs and renders a void paragraph if both the cite and quote are void 1`] = `
"<!-- wp:paragraph -->
<p></p>
<!-- /wp:paragraph -->"
`;

exports[`Quote can be converted to paragraphs and renders one paragraph block per <p> within quote 1`] = `
"<!-- wp:paragraph -->
Expand Down
6 changes: 6 additions & 0 deletions test/e2e/specs/splitting-merging.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
pressTimes,
pressWithModifier,
META_KEY,
waitForRichTextInitialization,
} from '../support/utils';

describe( 'splitting and merging blocks', () => {
Expand Down Expand Up @@ -132,7 +133,9 @@ describe( 'splitting and merging blocks', () => {
await pressWithModifier( META_KEY, 'b' );
await page.keyboard.press( 'ArrowRight' );
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();

await page.keyboard.press( 'Backspace' );

Expand Down Expand Up @@ -172,8 +175,11 @@ describe( 'splitting and merging blocks', () => {
await insertBlock( 'Paragraph' );
await page.keyboard.type( 'First' );
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();
await page.keyboard.press( 'Enter' );
await waitForRichTextInitialization();
await page.keyboard.type( 'Second' );
await page.keyboard.press( 'ArrowUp' );
await page.keyboard.press( 'ArrowUp' );
Expand Down
2 changes: 1 addition & 1 deletion test/e2e/support/utils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ async function login() {
* @return {Promise} Promise resolving once RichText is initialized, or is
* determined to not be a container of the active element.
*/
async function waitForRichTextInitialization() {
export async function waitForRichTextInitialization() {
const isInRichText = await page.evaluate( () => {
return !! document.activeElement.closest( '.editor-rich-text__tinymce' );
} );
Expand Down