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

Improve the Placeholder behavior to keep shown when focused. #8474

Merged

Conversation

huacnlee
Copy link
Contributor

@huacnlee huacnlee commented Nov 18, 2020

屏幕录制2020-11-18 11 53 44

@huacnlee huacnlee force-pushed the feat/better-placeholder-behavior branch from 1ac3cd9 to e6c2538 Compare November 18, 2020 06:08
@huacnlee huacnlee force-pushed the feat/better-placeholder-behavior branch from e6c2538 to a75f9df Compare November 19, 2020 03:11
@Reinmar Reinmar requested a review from oleq December 10, 2020 12:42
@oleq
Copy link
Member

oleq commented Dec 10, 2020

Hi @huacnlee!

Thanks for this contribution. It makes perfect sense that the editor placeholder should act the same as the <input> placeholder (it should disappear as you type, not when you focus). I checked your PR (editor root placeholder, the title plugin, RTL language support) and it all works fine. 

There's one problem, though: the image caption placeholder looks off:

I think we can correct it by using transform: translateX(-50%) for the .image figcaption.ck-placeholder::before:

which corresponds to what an <input> with a centered placeholder would look like (you can play with it live):

The question remains: do we want this @Reinmar @magda-chrzescian? This still looks off to me.

Other options

We could have something like below instead (.image figcaption.ck-placeholder::before { position: static }) but then:

  • 👎 it's incorrect that the caret appears right after the placeholder text instead of before it,
  • 👎 the caret would need to go back to the center of the <figcaption> when the user starts typing and that looks ugly (the caret jumping around like this)

Finally, the last way would be a perfectly centered placeholder text (like above) but a caret at the beginning of it:

  • 👍 this would at least look like the same as the editor placeholder,
  • ❓ I'm not sure this possible using CSS (I tried but failed),
  • 👎  the caret would need to go back to the center of the <figcaption> when the user starts typing and that looks ugly (the caret jumping around like this).

@huacnlee
Copy link
Contributor Author

huacnlee commented Dec 10, 2020

😂 That looks like too complex for me, I'm afraid that to have enough capacity to fix that.
I make this PR just wants fix the Placeholder behavior in editor initialization.

@magda-chrzescian
Copy link
Contributor

I would vote for center-center position. It's not perfect but I like that it seems consistent with the native input behavior and clear about the text alignment.

However, maybe the caption placeholder should be invisible on focus by default? I think that the main use case of focus-visible placeholder is when you want to prompt the user to fill the input (so it should be focused and should inform of the need to be filled) - it can be common regarding the content of editor, but rather rare in the case of image caption.

I really don't like the idea of caret jumping around right after the user starts to write. It would make an impression that something crashed and the user may stop writing for a second to check what happened. Besides, the initial position of caret can be confusing about how the text will be aligned.

@oleq
Copy link
Member

oleq commented Jan 15, 2021

However, maybe the caption placeholder should be invisible on focus by default?

I forked this PR in https://github.com/ckeditor/ckeditor5/compare/i/8689-smarter-editor-placeholder?expand=1 and I came up with a configuration for enablePlaceholder() that allows that.

2021-01-15 17 47 38

I also checked the Title plugin and it also looks fine.

@ckeditor/qa-team, can you take a look at https://github.com/ckeditor/ckeditor5/compare/i/8689-smarter-editor-placeholder?expand=1 and see if nothing else got broken (CF?) because of this? 🙏

@FilipTokarski
Copy link
Member

CKE 5

It seems to work fine, I didn't find any new bugs.

CF

I had errors at the beginning, but after locally merging latest master into the branch, everything works ok.
I see that apart from editor placeholder and Title plugin, this affects the behavior of annotations inputs too:

0_comment1.mp4

I'm not sure whether this was intended, but so far I didn't find any bugs related to this too.

@oleq
Copy link
Member

oleq commented Feb 2, 2021

Thanks @huacnlee for this contribution!

mhsdesign added a commit to neos/neos-ui that referenced this pull request Oct 9, 2024
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.
markusguenther added a commit to neos/neos-ui that referenced this pull request Oct 14, 2024
…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 <[email protected]>
laurahaenel pushed a commit to laurahaenel/neos-ui that referenced this pull request Oct 21, 2024
…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 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants