-
-
Notifications
You must be signed in to change notification settings - Fork 135
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
BUGFIX: 3557 placeholder plugin with autoparagraph false #3558
BUGFIX: 3557 placeholder plugin with autoparagraph false #3558
Conversation
So i was wondering why we even have our more or less hacky placeholder plugin. When we initially integrated CKEditor 5 #1942 its latest version was v10. And only with v12 the "ckeditor5-editor-decoupled" had native support for placeholders. see docs: https://ckeditor.com/docs/ckeditor5/16.0.0/api/module_core_editor_editorconfig-EditorConfig.html#member-placeholder and https://ckeditor.com/docs/ckeditor5/16.0.0/features/editor-placeholder.html luckily we use v12 in 7.3 and v16 in 8.3 so we can utilize this more stable native placeholder support as of right now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good and works in my tests, just one nitpick regarding a comment.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me…
42030fe
to
19402f2
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works like a charm! 👍
In Neos 7.3.17 and before the placeholder in ckeditor would not disappear when focused. And only when typing. By switching to the native placeholder feature of ckeditor in #3558 we introduced a regression as that implementation would hide the placeholder while just clicking into. This is especially noticeable for centered texts as the cursor needs one millisecond to adjust and its glitchy. This will eventually be fixed in Ckeditor 24 something. As a hotfix for our version 16 i introduce the patch of ckeditor/ckeditor5#8474 (without the css absolute change as that would make centered text uneditable). The mentioned fix was not merged directly into ckeditor but made way more complicated: ckeditor/ckeditor5#8867 We dont need the more complicated fix as we dont use the mentioned features of the placeholder in a image description and such.
…3864) * BUGFIX: Restore old Placeholder behavior to keep shown when focused In Neos 7.3.17 and before the placeholder in ckeditor would not disappear when focused. And only when typing. By switching to the native placeholder feature of ckeditor in #3558 we introduced a regression as that implementation would hide the placeholder while just clicking into. This is especially noticeable for centered texts as the cursor needs one millisecond to adjust and its glitchy. This will eventually be fixed in Ckeditor 24 something. As a hotfix for our version 16 i introduce the patch of ckeditor/ckeditor5#8474 (without the css absolute change as that would make centered text uneditable). The mentioned fix was not merged directly into ckeditor but made way more complicated: ckeditor/ckeditor5#8867 We dont need the more complicated fix as we dont use the mentioned features of the placeholder in a image description and such. * TASK: Move cursor before the placeholer * TASK: Hide break in placeholder context To move the cursor to the first position, the placeholder text is moved backwards. Therefore, a change with a break is disadvantageous. --------- Co-authored-by: Markus Günther <info@unikka.de>
…eos#3864) * BUGFIX: Restore old Placeholder behavior to keep shown when focused In Neos 7.3.17 and before the placeholder in ckeditor would not disappear when focused. And only when typing. By switching to the native placeholder feature of ckeditor in neos#3558 we introduced a regression as that implementation would hide the placeholder while just clicking into. This is especially noticeable for centered texts as the cursor needs one millisecond to adjust and its glitchy. This will eventually be fixed in Ckeditor 24 something. As a hotfix for our version 16 i introduce the patch of ckeditor/ckeditor5#8474 (without the css absolute change as that would make centered text uneditable). The mentioned fix was not merged directly into ckeditor but made way more complicated: ckeditor/ckeditor5#8867 We dont need the more complicated fix as we dont use the mentioned features of the placeholder in a image description and such. * TASK: Move cursor before the placeholer * TASK: Hide break in placeholder context To move the cursor to the first position, the placeholder text is moved backwards. Therefore, a change with a break is disadvantageous. --------- Co-authored-by: Markus Günther <info@unikka.de>
resolves: #3557
as the placeholder plugin was a bit hacky and broke in a regression from #3532
Because
editor.getData
now returned the new internal<neos-inline-wrapper>
tags. The preprocessing to strip them is now applied earlier, so other calls toeditor.getData
now get the expected results.also a problem is resolved that no placeholder is shown, if one chooses
h1
tag whenautoparagraph false
is enabledadditionally it cleans up the codebase and uses the native ckeditor placeholder plugin instead of our hacky home-brewed solution.
before and after:
Bildschirmaufnahme.2023-07-03.um.10.13.27.mov
Bildschirmaufnahme.2023-07-03.um.10.15.44.mov
Bildschirmaufnahme.2023-07-03.um.10.14.14.mov
What I did
How I did it
How to verify it