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

Jump to bottom

FEATURE: Serious ckeditor inlineMode isInline #3553

Draft
wants to merge 4 commits into
base: 8.4
Choose a base branch
from
Draft

FEATURE: Serious ckeditor inlineMode isInline #3553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2a0c238
TASK: Deprecated `autoparagraph: false` and rename it to `DisabledAut…
mhsdesign Jun 29, 2023
2c72e4c
FEATURE: Serious ckeditor inlineMode `isInline`
mhsdesign Jun 29, 2023
92e8e04
!!!TASK: Simplify legacy `autoparagraph: false` mode
mhsdesign Jun 29, 2023
ecc84ef
TASK: Fix and correct name of test
mhsdesign Jan 14, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file added .yarn/cache/@ckeditor-ckeditor5-editor-decoupled-patch-4948255868-bdaefd4a0b.zip
View file
Open in desktop
Binary file not shown.
2 changes: 2 additions & 0 deletions .yarnrc.yml
View file
Open in desktop
Expand Up @@ -35,3 +35,5 @@ changesetIgnorePatterns:
npmAlwaysAuth: false
npmAuthToken: "${NPM_TOKEN-}"
npmPublishAccess: "public"

patchFolder: "./patches"
4 changes: 2 additions & 2 deletions Tests/IntegrationTests/Fixtures/1Dimension/richTextEditor.e2e.js
View file
Open in desktop
Expand Up @@ -9,12 +9,12 @@ fixture`Rich text editor`
.beforeEach(beforeEach)
.afterEach(() => checkPropTypes());

test('Can crop an image', async t => {
test('Secondary RTE editor works', async t => {
const testContent = 'Test RTE content';
await Page.waitForIframeLoading(t);

const rteInspectorEditor = await ReactSelector('InspectorEditorEnvelope').withProps('id', 'rte');
const ckeContent = await Selector('.ck-content p');
const ckeContent = await Selector('.ck-content');
await t
.click(rteInspectorEditor.findReact('Button'));
await t
Expand Down
2 changes: 1 addition & 1 deletion Tests/IntegrationTests/SharedNodeTypesPackage/NodeTypes/Content/InlineHeadline.yaml
View file
Open in desktop
Expand Up @@ -21,7 +21,7 @@
inlineEditable: true
inline:
editorOptions:
autoparagraph: false
isInline: true
Copy link
Member Author

Choose a reason for hiding this comment

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

  • inlineMode?
  • inlineContentMode?
  • block = false

or decide in fusion?

Neos.Fusion:Editable {
   block = false
}

Copy link
Member Author

Choose a reason for hiding this comment

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

We decided to use the the same naming as in fusion: block false.

And its allowed to either specify it in the nodeType or via the rendering.

Following logic would apply:

    const isInlineMode = editorOptions?.block === false ||
         propertyDomNode.tagName === 'SPAN' ||
         propertyDomNode.tagName === 'H1' ||
         propertyDomNode.tagName === 'H2' ||
         propertyDomNode.tagName === 'H3' ||
         propertyDomNode.tagName === 'H4' ||
         propertyDomNode.tagName === 'H5' ||
         propertyDomNode.tagName === 'H6' ||
         propertyDomNode.tagName === 'P';

happy cases:

editorOptions: { } | { block: true } + Neos.Fusion:Editable { } | { block: true } -> <div>{isInlineMode: false}</div>
editorOptions: { } + Neos.Fusion:Editable { block: false } -> <span>{isInlineMode: true}</span>
editorOptions: { block: false } + Neos.Fusion:Editable { block: false } -> <span>{isInlineMode: true}</span>

but this would also lead to cases like this:

editorOptions: { block: false } + Neos.Fusion:Editable { } | { block: true } -> <div>{isInlineMode: true}</div>
editorOptions: { block: true } + Neos.Fusion:Editable { block: false } -> <span>{isInlineMode: false}</span>

So the the notation of block: false will always overrule the one from the Neos.Fusion:Editable. But the rendering will still be affected via Neos.Fusion:Editable
(i hope i have no logic error in my equations ^^)

# we show it also in the inspector so we can easily see the raw text content
inspector:
group: 'default'
Expand Down
3 changes: 2 additions & 1 deletion package.json
View file
Open in desktop
Expand Up @@ -10,7 +10,8 @@
"moment": "^2.20.1",
"vfile-message": "^2.0.2",
"isemail@3.2.0": "patch:isemail@npm:3.2.0#./patches/isemail-npm-3.2.0-browserified.patch",
"react-codemirror2@7.2.1": "patch:react-codemirror2@npm:7.2.1#./patches/react-codemirror2-npm-7.2.1-browserified.patch"
"react-codemirror2@7.2.1": "patch:react-codemirror2@npm:7.2.1#./patches/react-codemirror2-npm-7.2.1-browserified.patch",
"@ckeditor/ckeditor5-editor-decoupled@16.0.0": "patch:@ckeditor/ckeditor5-editor-decoupled@npm:16.0.0#./patches/@ckeditor-ckeditor5-editor-decoupled-npm-16.0.0-inline-mode.patch"
},
"scripts": {
"lint": "tsc --noemit && stylelint 'packages/*/src/**/*.css' && yarn eslint 'packages/*/src/**/*.{js,jsx,ts,tsx}'",
Expand Down
2 changes: 1 addition & 1 deletion packages/neos-ui-ckeditor5-bindings/package.json
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@
"@ckeditor/ckeditor5-alignment": "^16.0.0",
"@ckeditor/ckeditor5-basic-styles": "^16.0.0",
"@ckeditor/ckeditor5-core": "^16.0.0",
"@ckeditor/ckeditor5-editor-decoupled": "^16.0.0",
"@ckeditor/ckeditor5-editor-decoupled": "16.0.0",
"@ckeditor/ckeditor5-engine": "^16.0.0",
"@ckeditor/ckeditor5-essentials": "^16.0.0",
"@ckeditor/ckeditor5-heading": "^16.0.0",
Expand Down
12 changes: 11 additions & 1 deletion packages/neos-ui-ckeditor5-bindings/src/ckEditorApi.js
View file
Open in desktop
Expand Up @@ -44,6 +44,16 @@ export const createEditor = store => async options => {
propertyDomNode
});

const isInline = editorOptions?.isInline === true ||
propertyDomNode.tagName === 'SPAN' ||
propertyDomNode.tagName === 'H1' ||
propertyDomNode.tagName === 'H2' ||
propertyDomNode.tagName === 'H3' ||
propertyDomNode.tagName === 'H4' ||
propertyDomNode.tagName === 'H5' ||
propertyDomNode.tagName === 'H6' ||
propertyDomNode.tagName === 'P';

class NeosEditor extends DecoupledEditor {
constructor(...args) {
super(...args);
Expand All @@ -55,7 +65,7 @@ export const createEditor = store => async options => {
}

return NeosEditor
.create(propertyDomNode, ckEditorConfig)
.create(propertyDomNode, ckEditorConfig, /* this option is introduced via patch */ isInline)
.then(editor => {
editor.ui.focusTracker.on('change:isFocused', event => {
if (event.source.isFocused) {
Expand Down
15 changes: 0 additions & 15 deletions packages/neos-ui-ckeditor5-bindings/src/cleanupContentBeforeCommit.js
View file
Open in desktop
Expand Up @@ -4,20 +4,5 @@ export const cleanupContentBeforeCommit = content => {
if (content.match(/^<([a-z][a-z0-9]*)\b[^>]*>&nbsp;<\/\1>$/)) {
return '';
}

// We remove opening and closing span tags that are produced by the `DisabledAutoparagraphMode` plugin
if (content.startsWith('<span>') && content.endsWith('</span>')) {
const contentWithoutOuterSpan = content
.replace(/^<span>/, '')
.replace(/<\/span>$/, '');

if (contentWithoutOuterSpan.includes('<span>')) {
// In case there is still a span tag, we can be sure that the previously trimmed ones were belonging together,
// as it could be the case that multiple root paragraph/span elements were inserted into the ckeditor
// (which is currently not prevented if the html is modified from outside), so we will preserve the output.
return content;
}
return contentWithoutOuterSpan;
}
return content;
};
24 changes: 0 additions & 24 deletions packages/neos-ui-ckeditor5-bindings/src/cleanupContentBeforeCommit.spec.js
View file
Open in desktop
Expand Up @@ -8,27 +8,3 @@ test('remove empty nbsp', () => {
assertCleanedUpContent('<p>&nbsp;</p>', '');
assertCleanedUpContent('<span>&nbsp;</span>', '');
})

describe('ckeditor DisabledAutoparagraphMode hack, cleanup outer spans', () => {
test('noop', () => {
assertCleanedUpContent('<p></p>', '<p></p>');

assertCleanedUpContent('', '');

assertCleanedUpContent('<span><span>foo</span></span>', '<span><span>foo</span></span>');
})

test('cleanup single root <span>', () => {
assertCleanedUpContent('<span></span>', '');
assertCleanedUpContent('<span>foo</span>', 'foo');
})

test('cleanup multiple root <span>', () => {
assertCleanedUpContent('<span>foo</span><span>bar</span>', '<span>foo</span><span>bar</span>');
})

test('cleanup <span> root after other root', () => {
// in the case you had multiple paragraphs and a headline and switched to autoparagraph: false
assertCleanedUpContent('<h1>foo</h1><span>bar</span>', '<h1>foo</h1><span>bar</span>');
})
})
15 changes: 2 additions & 13 deletions packages/neos-ui-ckeditor5-bindings/src/manifest.config.js
View file
Open in desktop
Expand Up @@ -33,18 +33,6 @@ const addPlugin = (Plugin, isEnabled) => (ckEditorConfiguration, options) => {
return ckEditorConfiguration;
};

// If the editable is a span or a heading, we automatically disable paragraphs and enable the soft break mode
// Also possible to force this behavior with `autoparagraph: false`
const disableAutoparagraph = (editorOptions, {propertyDomNode}) =>
editorOptions?.autoparagraph === false ||
propertyDomNode.tagName === 'SPAN' ||
propertyDomNode.tagName === 'H1' ||
propertyDomNode.tagName === 'H2' ||
propertyDomNode.tagName === 'H3' ||
propertyDomNode.tagName === 'H4' ||
propertyDomNode.tagName === 'H5' ||
propertyDomNode.tagName === 'H6';

//
// Create richtext editing toolbar registry
//
Expand Down Expand Up @@ -102,7 +90,8 @@ export default ckEditorRegistry => {
//
config.set('essentials', addPlugin(Essentials));
config.set('paragraph', addPlugin(Paragraph));
config.set('disabledAutoparagraphMode', addPlugin(DisabledAutoparagraphMode, disableAutoparagraph));
// @deprecated
config.set('disabledAutoparagraphMode', addPlugin(DisabledAutoparagraphMode, (editorOptions) => editorOptions?.autoparagraph === false));
config.set('sub', addPlugin(Sub, $get('formatting.sub')));
config.set('sup', addPlugin(Sup, $get('formatting.sup')));
config.set('bold', addPlugin(Bold, $get('formatting.strong')));
Expand Down
6 changes: 2 additions & 4 deletions packages/neos-ui-ckeditor5-bindings/src/plugins/disabledAutoparagraphMode.js
View file
Open in desktop
@@ -1,10 +1,8 @@
import Plugin from '@ckeditor/ckeditor5-core/src/plugin';

/**
* HACK, since there is yet no native support for CKEditor 4 autoparagraph false
* see https://github.com/ckeditor/ckeditor5/issues/762
*
* We will try to find a serious alternative see https://github.com/neos/neos-ui/pull/3553
* Legacy HACK -> our previous "inlineMode" the `autoparagraph: false` mode (from CKEditor 4) for backwards compatibility
* @deprecated in favour of the serious "inlineMode"
*/
export default class DisabledAutoparagraphMode extends Plugin {
static get pluginName() {
Expand Down
2 changes: 1 addition & 1 deletion packages/neos-ui-ckeditor5-bindings/tests/manual/index.js
View file
Open in desktop
Expand Up @@ -34,7 +34,7 @@ createInlineEditor({
propertyDomNode: document.getElementById('input'),
propertyName: 'test',
editorOptions: {
autoparagraph: false,
isInline: true,
formatting: {
h1: true,
h2: true,
Expand Down
2 changes: 1 addition & 1 deletion packages/neos-ui-editors/src/Editors/Image/index.js
View file
Open in desktop
Expand Up @@ -57,7 +57,7 @@ export default class ImageEditor extends Component {
accept: PropTypes.string,

siteNodePath: PropTypes.string.isRequired,
focusedNodePath: PropTypes.string.isRequired,
focusedNodePath: PropTypes.string.isRequired
};

static defaultProps = {
Expand Down
64 changes: 64 additions & 0 deletions patches/@ckeditor-ckeditor5-editor-decoupled-npm-16.0.0-inline-mode.patch
View file
Open in desktop
@@ -0,0 +1,64 @@

fixes https://github.com/neos/neos-ui/issues/3545

diff --git a/src/decouplededitor.js b/src/decouplededitor.js
index 807e531075bdbded8a34a15ddc5fe50370e0fd61..73afd7a95c8d6b9adf16fe48bf4774d0cfade361 100755
--- a/src/decouplededitor.js
+++ b/src/decouplededitor.js
@@ -64,7 +64,7 @@ export default class DecoupledEditor extends Editor {
* {@link module:editor-balloon/ballooneditor~BalloonEditor.create `BalloonEditor.create()`}.
* @param {module:core/editor/editorconfig~EditorConfig} config The editor configuration.
*/
- constructor( sourceElementOrData, config ) {
+ constructor( sourceElementOrData, config, isInlineMode ) {
super( config );

if ( isElement( sourceElementOrData ) ) {
@@ -74,7 +74,25 @@ export default class DecoupledEditor extends Editor {

this.data.processor = new HtmlDataProcessor();

- this.model.document.createRoot();
+ if (isInlineMode === false) {
+ this.model.document.createRoot();
+ } else {
+ // patched ala https://github.com/ckeditor/ckeditor5/issues/762#issuecomment-370762111
+
+ // we define paragraph as root instead of $root. This will give us no outer tags out of the box and also disable the splitting
+ this.model.document.createRoot('paragraph');
+
+ // it is enforced that the root cannot be splitted, but to make this obvious for other plugins we set isLimit
+ this.on('ready', () => this.model.schema.extend('paragraph', {isLimit: true}));
+
+ // we redefine enter key to create soft breaks (<br>) instead of new paragraphs
+ this.editing.view.document.on('enter', (evt, data) => {
+ this.execute('shiftEnter');
+ data.preventDefault();
+ evt.stop();
+ this.editing.view.scrollToTheSelection();
+ }, {priority: 'high'});
+ }

const shouldToolbarGroupWhenFull = !this.config.get( 'toolbar.shouldNotGroupWhenFull' );
const view = new DecoupledEditorUIView( this.locale, this.editing.view, {
@@ -218,9 +236,10 @@ export default class DecoupledEditor extends Editor {
* {@link module:editor-decoupled/decouplededitorui~DecoupledEditorUI#getEditableElement `editor.ui.getEditableElement()`}.
*
* @param {module:core/editor/editorconfig~EditorConfig} [config] The editor configuration.
+ * @param {Boolean} isInlineMode Patched inline mode https://github.com/ckeditor/ckeditor5/issues/762#issuecomment-370762111
* @returns {Promise} A promise resolved once the editor is ready. The promise resolves with the created editor instance.
*/
- static create( sourceElementOrData, config = {} ) {
+ static create( sourceElementOrData, config = {}, isInlineMode ) {
return new Promise( resolve => {
const isHTMLElement = isElement( sourceElementOrData );

@@ -230,7 +249,7 @@ export default class DecoupledEditor extends Editor {
'editor-wrong-element: This type of editor cannot be initialized inside <textarea> element.', null );
}

- const editor = new this( sourceElementOrData, config );
+ const editor = new this( sourceElementOrData, config, isInlineMode );

resolve(
editor.initPlugins()
17 changes: 15 additions & 2 deletions yarn.lock
View file
Open in desktop
Expand Up @@ -1494,7 +1494,7 @@ __metadata:
languageName: node
linkType: hard

"@ckeditor/ckeditor5-editor-decoupled@npm:^16.0.0":
"@ckeditor/ckeditor5-editor-decoupled@npm:16.0.0":
version: 16.0.0
resolution: "@ckeditor/ckeditor5-editor-decoupled@npm:16.0.0"
dependencies:
Expand All @@ -1507,6 +1507,19 @@ __metadata:
languageName: node
linkType: hard

"@ckeditor/ckeditor5-editor-decoupled@patch:@ckeditor/ckeditor5-editor-decoupled@npm:16.0.0#./patches/@ckeditor-ckeditor5-editor-decoupled-npm-16.0.0-inline-mode.patch::locator=root-workspace-0b6124%40workspace%3A.":
version: 16.0.0
resolution: "@ckeditor/ckeditor5-editor-decoupled@patch:@ckeditor/ckeditor5-editor-decoupled@npm%3A16.0.0#./patches/@ckeditor-ckeditor5-editor-decoupled-npm-16.0.0-inline-mode.patch::version=16.0.0&hash=29fcf6&locator=root-workspace-0b6124%40workspace%3A."
dependencies:
"@ckeditor/ckeditor5-core": ^16.0.0
"@ckeditor/ckeditor5-engine": ^16.0.0
"@ckeditor/ckeditor5-ui": ^16.0.0
"@ckeditor/ckeditor5-utils": ^16.0.0
lodash-es: ^4.17.10
checksum: bdaefd4a0b870ce741ee8fa0963560b4f1e37b14f0fbc12b910667a8993aad186f57e64128ba5a9058ab44d9deea8b0e82b9ec9ed88c1d24afd2c80fea4a8dc9
languageName: node
linkType: hard

"@ckeditor/ckeditor5-engine@npm:^16.0.0":
version: 16.0.0
resolution: "@ckeditor/ckeditor5-engine@npm:16.0.0"
Expand Down Expand Up @@ -2325,7 +2338,7 @@ __metadata:
"@ckeditor/ckeditor5-alignment": ^16.0.0
"@ckeditor/ckeditor5-basic-styles": ^16.0.0
"@ckeditor/ckeditor5-core": ^16.0.0
"@ckeditor/ckeditor5-editor-decoupled": ^16.0.0
"@ckeditor/ckeditor5-editor-decoupled": 16.0.0
"@ckeditor/ckeditor5-engine": ^16.0.0
"@ckeditor/ckeditor5-essentials": ^16.0.0
"@ckeditor/ckeditor5-heading": ^16.0.0
Expand Down