-
Notifications
You must be signed in to change notification settings - Fork 47.6k
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
Use the native beforeinput
event if it's supported
#11211
Comments
Any word from any maintainers about this? Just curious what the opinion is. Thanks! |
@ianstormtaylor do you know if there's any reliable way to detect if
How hard do you think it would be to polyfill those spec'd properties for other browsers? We usually don't include event properties if we can't provide them consistently. |
@aweary why is it that the current logic doesn't work? We use what seems like a similar detection method for it right now. (Although I have to be honest, I'm not sure if it's as reliable as needed by React with it's much broader browser targets.) As for polyfilling the spec'd properties. I'm not sure, but I think it would be hard. I'd actually advise that instead of trying to polyfill the properties as top-level properties on the synthetic events, that users reach into the But I think there might be a larger issue that React's I'm not super familiar with React's events, but it seems like the existing |
@ianstormtaylor I tested https://jsfiddle.net/t4dsqLj9/
That seems fine to me until it has better browser support.
If it doesn't implement the spec'd behavior we could potentionally see if we could update the event plugin so it does? I don't think the inconsistencies are intentional. |
@aweary ah gotcha, it might be because Chrome doesn't implement the spec fully yet? Not sure on that one. Updating could be good! I don't know enough about the internals to be able to suggest things there I think. The issue is that right now it's text input only, whereas the newer specs result it in being used for events that aren't necessarily character-inserting. |
@aweary While the hack slate uses to test for beforeinput support now generates a false negative for chrome: const testEl = window.document.createElement('div');
testEl.contentEditable = true;
'onbeforeinput' in testEl; // => false in chrome 66.0.3359.181, despite support ...might something that tests for the 'inputType' in (new InputEvent('input'));
It might be further splayed out to handle arbitrarily old browsers with something like this, too: 'inputType' in (window.InputEvent ? new InputEvent('input') : {}); |
@johan you're right. That's interesting. I did a little investigating, and it looks like this tests end up as: const event = window.InputEvent ? new InputEvent('input') : {}
const hasLevel1 = 'inputType' in event const element = document.createElement('div')
element.contentEditable = true
const hasLevel2 = 'onbeforeinput' in element Depending on whether you're testing for Input Events Level 1 support ( Seems like React doesn't need to distinguish for its purposes, so the Level 1 test is the way to go. (Also because the Level 2 test seems a bit fishy anyways.) |
We've been taking a look at this issue because we're experiencing a problem with cancelling I will probably be opening up a PR to React to implement this behavior, but React has such a huge net of supported browsers that I don't know how to go about determining that adding this behavior won't break anything... |
@ianstormtaylor Now that Firefox released a version 66 which tests positive for that
Be careful what you test for, and all that. 🙈 |
FYI: When Firefox ship Perhaps, Mozilla won't ship I think that ideal feature detection of whether Unfortunately, there is no way to detect whether the browser supports Input Events Level 1 or Level 2 until receiving Level 2 specific |
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution. |
Not stale. This is still frustrating to have to work around. |
Yeah, it would be delightful not to need to create a ref and |
This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment! |
This is still affecting draft-js |
I started to look into this today and would like to get some thoughts on #19554. Specifically in regards to the detection mechanism. I tried detecting it via |
To people who expressed frustration in this thread — I totally feel you but the most productive way to move this forward is to share your expertise in this and help us figure out a reliable detection mechanism. Sending a PR with that would have moved the issue further much earlier. |
Thanks for working on this! I'll add some notes on the PR.
You can do it on a newly created div, on which you have set the Your current check for |
FYI: Firefox Nightly does now support |
@masayuki-nakano Thank you for the update and for all the work you've put into this. The support for |
@masayuki-nakano is away for the next couple of days so let me add a couple of details and he can provide more information when he gets back. The current status is that If React could start using Firefox's native |
Thank you @birtles for the explanation. Yes, we'd love to ship it as far as possible, but after shipping it, the behavior change becomes too risky because such things may require UA string check if they are caused by a bug of Firefox or just undefined incompatible things with the others. So, for reducing such risk, we'd love you to feedback with testing with Firefox Nightly. Especially we'd love to know what are unacceptable differences from the other browsers. I guess that such things mainly exist at the result of |
@masayuki-nakano I tried out the latest nightly of FF and it's awesome. For context, we're looking to start using native On a side note, I noticed that FF Nightly copies Chromium in that |
Well, they are only in Level 2, but the Level 2's definition around composition may break a lot of existing web apps due to the incompatible things. That's the reason why I considered that Gecko follows Blink (i.e., meaning supports only Level 1 by default). If you just want FYI1: Firefox has experimental implementation about Input Events Level 2 behind a pref, FYI2: If Facebook use |
@masayuki-nakano Let me give Level 2 a try (it's a bit confusing why the flag says level 1 though). What we want to do is allow native composition – i.e. not cancelling it, as you can't really cancel it on Blink. Instead we want to know about the composed text. Only WebKit currently fires What differences in |
The pref means that limits the behavior for conforming to Level 1's spec. Therefore, it sounds odd.
Yes.
WebKit supports only And what you want is exactly the spec issue. I'll ping to the guys.
Thank you for the information. I've not tested it, I'll check it. The main differences of Similarly, there is same difference when you type Finally, deleting an atomic element like |
Ran into this issue today as well. Unfortunately, React's It's likely that changing the behavior to match the spec would break a bunch of existing React components that rely on |
@devongovett It's something I thought long and hard about, especially as I'm currently working on a text editor at Facebook and obviously want to leverage the benefits of It's more likely that we'd add support to React for |
@masayuki-nakano Have there been any updates or updates regarding this feature shipping? We're actively looking to adopt We noticed that We're looking to ship this new feature by the end of this half, so any estimations would be great. |
@trueadm Hi, we've decided last week that it's shipped in 87 unless we'd get some bug reports which can block the release. And thank you for the feedback about the |
Oops, 87 will be shipped March, 23. |
@trueadm Just curious, if that's no secret, are you working on something other than Draft.js that would be open-sourced at some point or is it purely an internal project? |
@JanJakes Yes, I'm working on something other than Draft.js. I expect the project to be eventually open-sourced at some point too, although no guarantees or dates as it's too early :) |
@masayuki-nakano Does it mean if I set |
This creates webcompat issues for Firefox. |
Still having this issue; anyone else? I need to access the selectionStart and selectionEnd of an Anyone else willing to share their workarounds? |
Do you want to request a feature or report a bug?
Improvement.
What is the current behavior?
Right now, the synthetic
onBeforeInput
event is being created based on two other events:textInput
when possible—which is in Webkit.keypress
as a fallback.But these days in Chrome, Safari and Opera the spec'd
beforeinput
event is available and actually fires. And when it does, it includes other spec'd properties which can be extremely helpful:inputType
tells you whether the event is inserting text, replacing text, inserting a line break, etc.getTargetRanges()
tells you where the input is taking place in the DOM.Right now this information isn't exposed, because even if the browser supports
beforeinput
, it's not being checked for.What is the expected behavior?
Instead React should treat
textInput
as a slightly-preferred fallback for nativebeforeinput
support, but addbeforeinput
as the true goal. So we'd end up with a fallback stack of:beforeinput
textInput
keypress
Which guarantees that the
nativeEvent
will always be the most spec'd and have the most relevant information associated with it.The
beforeinput
event's extra properties are critical incontenteditable
situations, when you want to prevent the default browser behavior from firing but perform the logic on an internal model instead. (I'm looking to do this for Slate.)Without that extra information you have to fallback to hackier behavior—allowing the event to occur, trying to parse the DOM for what the change was, then re-rendering to remove it, etc. I want to avoid this on the more modern browsers, because it results in reduced performance.
There is another situation that this fixes, which is that spellcheck right now doesn't trigger React's
onBeforeInput
handler, even though modern browsers fire thebeforeinput
event, because it's not being listened for right now.The text was updated successfully, but these errors were encountered: