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

add support for SyntheticKeyboardEvent#isComposing #13104

Open
mattkrick opened this issue Jun 24, 2018 · 18 comments · May be fixed by #26254
Open

add support for SyntheticKeyboardEvent#isComposing #13104

mattkrick opened this issue Jun 24, 2018 · 18 comments · May be fixed by #26254

Comments

@mattkrick
Copy link

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

@gaearon
Copy link
Collaborator

gaearon commented Aug 2, 2018

Want to send a PR?

@craigkovatch
Copy link

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
Copy link

craigkovatch commented Sep 25, 2019

@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"?

@stale
Copy link

stale bot commented Jan 9, 2020

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 stale bot added the Resolution: Stale Automatically closed due to inactivity label Jan 9, 2020
@craigkovatch
Copy link

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

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 9, 2020
@stale
Copy link

stale bot commented Apr 8, 2020

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 stale bot added the Resolution: Stale Automatically closed due to inactivity label Apr 8, 2020
@craigkovatch
Copy link

bump

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 8, 2020
@eps1lon
Copy link
Collaborator

eps1lon commented Apr 8, 2020

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.

@craigkovatch
Copy link

@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

@eps1lon
Copy link
Collaborator

eps1lon commented Apr 8, 2020

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.

@stale
Copy link

stale bot commented Jul 11, 2020

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 stale bot added the Resolution: Stale Automatically closed due to inactivity label Jul 11, 2020
@eps1lon
Copy link
Collaborator

eps1lon commented Jul 11, 2020

bump

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jul 11, 2020
@craigkovatch
Copy link

craigkovatch commented Sep 12, 2020

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}
  />;
});

@stale
Copy link

stale bot commented Dec 25, 2020

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 stale bot added the Resolution: Stale Automatically closed due to inactivity label Dec 25, 2020
@craigkovatch
Copy link

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

@stale stale bot removed the Resolution: Stale Automatically closed due to inactivity label Jan 16, 2021
@craigkovatch
Copy link

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

@Austaras
Copy link

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

@craigkovatch
Copy link

craigkovatch commented Oct 11, 2022 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants