TASK: Apply suggestion from code review

neos · Jul 3, 2023 · 19402f2 · 19402f2
1 parent 1234d9a
commit 19402f2
Show file tree

Hide file tree

Showing 5 changed files with 40 additions and 49 deletions.
diff --git a/packages/neos-ui-ckeditor5-bindings/src/cleanupContentBeforeCommit.spec.js b/packages/neos-ui-ckeditor5-bindings/src/cleanupContentBeforeCommit.spec.js
@@ -1,4 +1,4 @@
-import {cleanupContentBeforeCommit} from './cleanupContentBeforeCommit'
+import {cleanupContentBeforeCommit} from './cleanupContentBeforeCommit';
 
 const assertCleanedUpContent = (input, expected) => {
     expect(cleanupContentBeforeCommit(input)).toBe(expected);

diff --git a/packages/neos-ui-ckeditor5-bindings/src/manifest.config.js b/packages/neos-ui-ckeditor5-bindings/src/manifest.config.js
@@ -81,11 +81,18 @@ export default ckEditorRegistry => {
 
     //
     // Base CKE configuration
+    // - configuration of language
+    // - and placeholder feature see https://ckeditor.com/docs/ckeditor5/16.0.0/api/module_core_editor_editorconfig-EditorConfig.html#member-placeholder
     //
-    config.set('baseConfiguration', (ckEditorConfiguration, {userPreferences}) => {
-        return Object.assign(ckEditorConfiguration, {
+    config.set('baseConfiguration', (ckEditorConfiguration, {globalRegistry, editorOptions, userPreferences}) => {
+        const i18nRegistry = globalRegistry.get('i18n');
+        const placeholder = $get('placeholder', editorOptions);
+        return {
+            ...ckEditorConfiguration,
+            // stripTags, because we allow `<p>Edit text here</p>` as placeholder for legacy
+            placeholder: placeholder ? stripTags(i18nRegistry.translate(placeholder)) : undefined,
             language: String($get('interfaceLanguage', userPreferences))
-        });
+        };
     });
 
     //
@@ -144,21 +151,5 @@ export default ckEditorRegistry => {
             ]}
     }));
 
-    //
-    // @see https://ckeditor.com/docs/ckeditor5/16.0.0/api/module_core_editor_editorconfig-EditorConfig.html#member-placeholder
-    //
-    config.set('placeholder', (config, {globalRegistry, editorOptions}) => {
-        const i18nRegistry = globalRegistry.get('i18n');
-        const placeholder = $get('placeholder', editorOptions);
-        if (!placeholder) {
-            return config;
-        }
-        return {
-            ...config,
-            // stripTags, because we allow `<p>Edit text here</p>` as placeholder for legacy
-            placeholder: stripTags(i18nRegistry.translate(placeholder))
-        };
-    });
-
     return config;
 };
diff --git a/packages/neos-ui-ckeditor5-bindings/src/plugins/cleanupNeosInlineWrapper.js b/packages/neos-ui-ckeditor5-bindings/src/plugins/cleanupNeosInlineWrapper.js
@@ -0,0 +1,27 @@
+/**
+ * We remove opening and closing span tags that are produced by the inlineMode plugin
+ *
+ * @private only exported for testing
+ * @param {String} content
+ */
+export const cleanupNeosInlineWrapper = content => {
+    if (content.includes('<neos-inline-wrapper>')) {
+        let contentWithoutOuterInlineWrapper = content;
+
+        if (content.startsWith('<neos-inline-wrapper>') && content.endsWith('</neos-inline-wrapper>')) {
+            contentWithoutOuterInlineWrapper = content
+                .replace(/^<neos-inline-wrapper>/, '')
+                .replace(/<\/neos-inline-wrapper>$/, '');
+        }
+
+        if (contentWithoutOuterInlineWrapper.includes('<neos-inline-wrapper>')) {
+            // in the case, multiple root paragraph elements were inserted into the ckeditor (wich is currently not prevented if the html is modified from outside)
+            // we have multiple root elements of type <neos-inline-wrapper>. We will convert all of them into spans.
+            return content
+                .replace(/<neos-inline-wrapper>/g, '<span>')
+                .replace(/<\/neos-inline-wrapper>/g, '</span>');
+        }
+        return contentWithoutOuterInlineWrapper;
+    }
+    return content;
+};
diff --git a/...5-bindings/src/plugins/inlineMode.spec.js → .../plugins/cleanupNeosInlineWrapper.spec.js b/...5-bindings/src/plugins/inlineMode.spec.js → .../plugins/cleanupNeosInlineWrapper.spec.js
@@ -1,4 +1,4 @@
-import {cleanupNeosInlineWrapper} from './inlineMode'
+import {cleanupNeosInlineWrapper} from './cleanupNeosInlineWrapper';
 
 const assertCleanedUpContent = (input, expected) => {
     expect(cleanupNeosInlineWrapper(input)).toBe(expected);

diff --git a/packages/neos-ui-ckeditor5-bindings/src/plugins/inlineMode.js b/packages/neos-ui-ckeditor5-bindings/src/plugins/inlineMode.js
@@ -1,4 +1,5 @@
 import Plugin from '@ckeditor/ckeditor5-core/src/plugin';
+import {cleanupNeosInlineWrapper} from './cleanupNeosInlineWrapper';
 
 /**
  * HACK, since there is yet no native support
@@ -40,31 +41,3 @@ export default class InlineMode extends Plugin {
         }, {priority: 'high'});
     }
 }
-
-/**
- * We remove opening and closing span tags that are produced by the inlineMode plugin
- *
- * @private only exported for testing
- * @param {String} content
- */
-export const cleanupNeosInlineWrapper = content => {
-    if (content.includes('<neos-inline-wrapper>')) {
-        let contentWithoutOuterInlineWrapper = content;
-
-        if (content.startsWith('<neos-inline-wrapper>') && content.endsWith('</neos-inline-wrapper>')) {
-            contentWithoutOuterInlineWrapper = content
-                .replace(/^<neos-inline-wrapper>/, '')
-                .replace(/<\/neos-inline-wrapper>$/, '');
-        }
-
-        if (contentWithoutOuterInlineWrapper.includes('<neos-inline-wrapper>')) {
-            // in the case, multiple root paragraph elements were inserted into the ckeditor (wich is currently not prevented if the html is modified from outside)
-            // we have multiple root elements of type <neos-inline-wrapper>. We will convert all of them into spans.
-            return content
-                .replace(/<neos-inline-wrapper>/g, '<span>')
-                .replace(/<\/neos-inline-wrapper>/g, '</span>');
-        }
-        return contentWithoutOuterInlineWrapper;
-    }
-    return content;
-};