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

fix(events): map onFocusIn/Out to correct events #2745

Merged
merged 5 commits into from
Dec 16, 2020

Conversation

claviska
Copy link
Contributor

@claviska claviska commented Nov 19, 2020

Fixes #2628.

Support for onFocusIn and onFocusOut were added in #2437. These events are supported by all browsers and are widely used, but they're not defined as properties in the spec yet. As such, they don't exist as properties on the window object.

The end result is the attached listeners being focusIn and focusOut instead of focusin and focusout. This PR adds an array of event transformations to fall back on for edge case events such as these.

Update: we've decided to change the types to onFocusin and onFocusout so we don't modify the runtime. Implemented in 513c571.

@claviska
Copy link
Contributor Author

I took a look at what Preact does and it turns out they don't support either event and neither does React. Notably, the reasoning is:

The supported part is actually driven by the fact that the event exists on the Global Event Interface and they do not. Just try it in your console 'onfocusin' in window.

However, in both Preact and React the onFocus and onBlur methods bubble up, whereas natively (and in Stencil) they do not. It's worth mentioning that, currently, you can make this work with @ts-ignore and lowercasing:

render() {
  return (
    // @ts-ignore
    <div onFocusin={() => console.log('focus')} onFocusout={() => console.log('blur')}>
      <div contenteditable>Edit me</div>
    </div>
  );
}

But this doesn't seem like a good solution. 😕

@adamdbradley
Copy link
Contributor

I think the best solution would be to change the JSX types from onFocusIn to onFocusin. It'd force the correct case, and we wouldn't have to add to the runtime for these special cases.

@WickyNilliams
Copy link
Contributor

I'd be happy with the onFocusin/onFocusout approach

@claviska claviska marked this pull request as ready for review December 16, 2020 19:55
@claviska claviska self-assigned this Dec 16, 2020
@adamdbradley adamdbradley merged commit 2dc930f into master Dec 16, 2020
@adamdbradley adamdbradley deleted the focusin-focusout-fix branch December 16, 2020 20:18
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.

issues with focusin event in JSX
3 participants