Rich text: remove placeholder on composition start #41970
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Size Change: +29 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
dmsnell
approved these changes
Jun 27, 2022
There was a problem hiding this comment.
There's a comment in line 71 or the source stating Safari doesn't support isComposing, but I believe that's only for the InputEvent, since it works for KeyboardEvent. Not necessary for this PR, but I found it slightly confusing when trying to understand the code and the change.
This looks great and I don't see an easy way in which it will break. Did you have any reluctance holding it as a draft? (I'm approving even though it's a draft, which you are free to ignore and I didn't mean to jump in too quickly on it).
| // during composition, the placeholder doesn't get removed. There's | ||
| // no need to re-add it, when the value is updated on compositionend | ||
| // it will be re-added when the value is empty. | ||
| element.querySelector( `[${ PLACEHOLDER_ATTR_NAME }]` ).remove(); |
There was a problem hiding this comment.
this seems quite elegant and works for the input cases I tested, namely Japanese entry and Kanji selection.
ellatrix
force-pushed
the
try/remove-placeholder-on-composition
branch
from
e98f5c2 to
37d5208
Compare
Right, so we're storing
No, it just seems like Github made that the default now and I didn't check. |
7 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
When composing characters in rich text, the placeholder remains in place because the rich text value doesn't update internally. We can't update the value internally because that would destroy the state of composition set by the browser (because we modify the dom). So the only option we have is to remove it from the dom on composition start. An ugly solution, but can't think of another way right now.
Why?
How?
Testing Instructions
Screenshots or screencast