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

Optionally use native 'beforeinput' events instead of React's synthetic event #84

Merged
merged 2 commits into from
Feb 21, 2019

Conversation

ShannonLCapper
Copy link

React currently does not support using the native beforeinput event supported by some browsers and instead polyfills the event using either textInput or keypress events (more context here). beforeinput events with Level 2 Input support (meaning that they fire and are cancellable) currently only exists in Safari (more context here)

However, we have a bug that specifically repros in the version of Safari that runs in Office Add-ins for Mac--calling preventDefault() on a textInput event will cause the system OS alert noise.

This PR optionally bypasses React's synthetic onBeforeInput event (which uses textInput events under the covers in Safari) and allows consumers to specify that we should instead use the native beforeinput event directly if they're supported in the browser. This is the same thing Slate does, and allows us to get around our very specific error case in Outlook for Mac. However, since this opens us up to opportunities for unexpected behavior, this is opt-in only to reduce our risk surface-area.

Copy link

@max-winderbaum max-winderbaum left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@ShannonLCapper ShannonLCapper merged commit df3ea0c into release Feb 21, 2019
@ShannonLCapper ShannonLCapper deleted the topic-native-before-input branch February 21, 2019 00:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants