The Wayback Machine - https://web.archive.org/web/20200703185130/https://github.com/preactjs/preact/issues/1621
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

Should onFocus/onBlur bubble or not? #1621

Open
maranomynet opened this issue May 10, 2019 · 12 comments
Open

Should onFocus/onBlur bubble or not? #1621

maranomynet opened this issue May 10, 2019 · 12 comments
Labels

Comments

@maranomynet
Copy link

@maranomynet maranomynet commented May 10, 2019

I'm confused about whether onFocus/onBlur events should bubble in preact, like they do in React.

In this example using preact 10.0.0-beta.1 they don't bubble https://codesandbox.io/embed/5wxk9zk7lx

...whereas in the Preact repl they do bubble https://preactjs.com/repl

export default () =>(
  <div onFocus={() => console.log("FOCUS")}>
    <div>
      Focus this input
      <br />
      <input />
    </div>
  </div>
);

So which example is correct Preact behavior?

If they should not bubble, then that fact really should be covered on the Differences to React page

... or even receive its own page about events and how onChange is different and how only some DOM event prop names may be camelCased (onClick), while others must be lowercased (like: onfocusin)

Then also the REPL should be fixed to behave like "normal" Preact.

@maranomynet
Copy link
Author

@maranomynet maranomynet commented May 10, 2019

Related to #1611

@JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented May 10, 2019

At this moment the Repl is showing the latest stable version of Preact (and so are the docs), 10 is still in beta and this could be a quirk that we should fix. Thanks for making us aware.

@maranomynet
Copy link
Author

@maranomynet maranomynet commented May 10, 2019

@JoviDeCroock But the plot thickens:

It seems the same with preact@8.4.2 https://codesandbox.io/s/4qvw991wk9

@maranomynet
Copy link
Author

@maranomynet maranomynet commented May 10, 2019

...and with preact@8.3.1 https://codesandbox.io/s/4jylo618q0

No bubbling of onFocus

@maranomynet
Copy link
Author

@maranomynet maranomynet commented May 10, 2019

However I see onFocus bubbling in preact@7.2.1 – but not after that.

@robertknight
Copy link
Member

@robertknight robertknight commented May 10, 2019

Given that there are native focusin and focusout events which do bubble, I think the behaviour in keeping with the "closer to the metal" philosophy would be to avoid special-casing this event and require use of onFocusIn or onFocusOut props if users want bubbling.

A counter-argument is React compatibility if there are important third-party components which rely on the behaviour of onFocus. Potentially that could be addressed in the preact/compat package though.

@maranomynet
Copy link
Author

@maranomynet maranomynet commented May 11, 2019

I think that approach/attitude would make sense.

But the feature of onFocus/onBlur not bubbling would have to be retrofitted into the Changelog as breaking changes in version 8.0.0.

Additionally Preact's "close to the metal" event model would need to be documented a whole lot better than it is today.
I love Preact's hands-off approach to onChange, for example, but it's frustrating to have to figure it out the hard way when your React-influenced components "don't work".

@robertknight
Copy link
Member

@robertknight robertknight commented May 11, 2019

The change to the behaviour of onFocus and onBlur happened in 727f036#diff-4935bb00e75b7854383877cbfd9d770f. I haven't checked the changelog but if it wasn't mentioned, that was an oversight.

Additionally Preact's "close to the metal" event model would need to be documented a whole lot better than it is today.

I agree. The section on event handling in https://preactjs.com/guide/differences-to-react is currently very brief.

@JoviDeCroock
Copy link
Member

@JoviDeCroock JoviDeCroock commented May 12, 2019

Hmm, so we can divide this into two issues I think?

On one hand we need to expand the docs to better reflect the differences in bubbling, ... compared to react.
On the other hand we should discuss whether or not we should also have this behavior in compat (personally think this would produce a lot of bundle size)

@robertknight
Copy link
Member

@robertknight robertknight commented May 17, 2019

On the other hand we should discuss whether or not we should also have this behavior in compat

I don't know if there are any already agreed-upon criteria for how far preact/compat should go to emulate React. A suggestion I have is that it would depend on whether the behaviour is critical for a popular third-party library, since that is a major use case for using preact/compat and the one where the app developer can't just make a simple change to their own code.

@developit
Copy link
Member

@developit developit commented May 24, 2019

The REPL is using Preact 7.

@developit
Copy link
Member

@developit developit commented May 24, 2019

I think this behaviour got introduced by accident as a side effect of supporting on**Capture events.

FWIW a compat fix would be to transform props.onFocus to props.onFocusCapture.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
4 participants
You can’t perform that action at this time.