react icon indicating copy to clipboard operation
react copied to clipboard

add support for SyntheticKeyboardEvent#isComposing

Open mattkrick opened this issue 7 years ago • 18 comments

Do you want to request a feature or report a bug? Bug

What is the current behavior? Synthetic keyboard events do not contain isComposing. They should if the value is true, per the w3 spec 4.7.5: https://www.w3.org/TR/uievents/#events-compositionevents

What is the expected behavior? event.isComposing === event.nativeEvent.isComposing

SyntheticKeyboardEvent#isComposing is true when a keydown even is fired after compositionstart and before compositionend.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React? all versions, up through at least 16.4.1

mattkrick avatar Jun 24 '18 21:06 mattkrick

Want to send a PR?

gaearon avatar Aug 02 '18 17:08 gaearon

This is native-unsupported on IE and Safari < 10.1. Would need to be polyfilled, although it is an extremely simple polyfill: true if seen after compositionStart but before compositionEnd, otherwise false.

craigkovatch avatar Mar 14 '19 23:03 craigkovatch

@gaearon I would like to work on a PR for this. Copying the field from the nativeEvent is easy, polyfilling for IE11 (+old Safari) is not as would need to listen for compositionstart and compositionend events on relevant event targets and save that state somewhere for the KeyboardEvent to read. Do you have any pointers on existing features in React that might follow this pattern of "save data from some native event and use it to polyfill a field in another (synthetic) event"?

craigkovatch avatar Sep 25 '19 21:09 craigkovatch

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.

stale[bot] avatar Jan 09 '20 20:01 stale[bot]

Bump -- still hoping for some guidance from React team re: previous comment

craigkovatch avatar Jan 09 '20 21:01 craigkovatch

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 additional information, please include with in your comment!

stale[bot] avatar Apr 08 '20 21:04 stale[bot]

bump

craigkovatch avatar Apr 08 '20 21:04 craigkovatch

Do you have any pointers on existing features in React that might follow this pattern of "save data from some native event and use it to polyfill a field in another (synthetic) event"?

Isn't relatedTarget in blur (or focus?) used from focusout? Not sure if this applies here though.

eps1lon avatar Apr 08 '20 21:04 eps1lon

@eps1lon possibly! Could you point me at anywhere in particular? I am motivated to help with this but 100% noob on the React codebase.

Also it would be news to me if React was polyfilling relatedTarget in blur events -- that would be real nice since it's missing in IE11

craigkovatch avatar Apr 08 '20 21:04 craigkovatch

possibly! Could you point me at anywhere in particular? I am motivated to help with this but 100% noob on the React codebase.

I'm not familiar with most of react-dom, sorry.

Also it would be news to me if React was polyfilling relatedTarget in blur events -- that would be real nice since it's missing in IE11

If I remember correctly polyfilling was the actual issue.

eps1lon avatar Apr 08 '20 22:04 eps1lon

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!

stale[bot] avatar Jul 11 '20 02:07 stale[bot]

bump

eps1lon avatar Jul 11 '20 08:07 eps1lon

Here is what I use to work around this issue. My "cloning" of the event via ...e in here might be dangerous for React internal reasons that are not known to me, use at your own risk:

const isIE = /Trident/.test(navigator.userAgent);
export const IMECompatibleInput = React.forwardRef((props, ref) => {
  const isComposing = React.useRef(false);
  return <input
    {...props}
    onBlur={props.onBlur && (e => {
      e.relatedTarget = isIE ? e.relatedTarget || document.activeElement : e.relatedTarget;
      props.onBlur(e);
    })}
    onChange={props.onChange && (e => props.onChange({ ...e, isComposing: isComposing.current }))}
    onCompositionEnd={e => {
      isComposing.current = false;
      props.onChange?.({ ...e, isComposing: false });
      props.onCompositionEnd?.(e);
    }}
    onCompositionStart={e => {
      isComposing.current = true;
      props.onCompositionStart?.(e);
    }}
    ref={ref}
  />;
});

craigkovatch avatar Sep 12 '20 03:09 craigkovatch

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!

stale[bot] avatar Dec 25 '20 13:12 stale[bot]

Still an issue, although less interesting the older it gets (e.g. we are dropping support for IE11 in a few months)

craigkovatch avatar Jan 16 '21 17:01 craigkovatch

Turns out my workaround breaks the ability to call stopPropagation on the synthetic change event later.

craigkovatch avatar May 11 '21 00:05 craigkovatch

If I'm reading this correctly, can we just copy from nativeEvent and ignore IE?

Austaras avatar Jul 11 '22 09:07 Austaras

Works for me, IE is dead

-Craig (mobile)


From: Austaras @.> Sent: Monday, July 11, 2022 2:47:11 AM To: facebook/react @.> Cc: Craig Kovatch @.>; Comment @.> Subject: Re: [facebook/react] add support for SyntheticKeyboardEvent#isComposing (#13104)

If I'm reading thishttps://create-react-app.dev/docs/supported-browsers-features/ correctly, can we just copy from nativeEvent and ignore IE?

— Reply to this email directly, view it on GitHubhttps://github.com/facebook/react/issues/13104#issuecomment-1180187838, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ACTWJEIVFYIWLRP5LMXNAUTVTPUR7ANCNFSM4FGVIEHQ. You are receiving this because you commented.Message ID: @.***>

craigkovatch avatar Oct 11 '22 08:10 craigkovatch