facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
228.53k stars 46.77k forks source link

No blur event fired when button is disabled/removed #9142

Open OliverJAsh opened 7 years ago

OliverJAsh commented 7 years ago

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

What is the current behavior? When a focussed button becomes disabled, React does not dispatch a blur event.

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem via https://jsfiddle.net or similar (template: https://jsfiddle.net/reactjs/69z2wepo/).

  1. Attach a blur event to a button
  2. Focus the button
  3. Make the button disabled or remove it from the DOM

What is the expected behavior? A blur event will be dispatched.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React? 15.1.0, not sure if it worked in previous versions.

Isolated test case: http://jsbin.com/fuvite/1/edit?html,css,js,output

OliverJAsh commented 7 years ago

Hmm, I don't want to mutate the DOM. Disabled is a field in my state that I want to use when rendering

jddxf commented 7 years ago

Just use setState.See http://codepen.io/inamessnow/pen/XMMYLK?editors=0010

aweary commented 7 years ago

Calling ReactDOM.render again should act identically to renders triggered from setState. React does not re-create the element in this case (watch it in devtools)

@OliverJAsh I tried your demo and I see both blur and blurCapture events working just fine:

screen shot 2017-03-19 at 12 28 34 am

Using Chrome 56. Is there a specific browser you were having this issue with? Also, it sounds like you are using setState to trigger the render in your actual application, but if you are using ReactDOM.render to re-render an application we do recommend avoiding that in most cases.

OliverJAsh commented 7 years ago

@aweary Please see the gif demo below. In the demo, you can see my vanilla event listener is logged, but the React blur and blurCapture event listeners are not (when the button becomes disabled). The updated code is at http://jsbin.com/loqoquzuye/1/edit?js,output

untitled

aweary commented 7 years ago

@OliverJAsh can you verify what browser you're using? I can't reproduce in Chrome 56, but I can reproduce it in Chrome 57. It looks to be browser-specific.

Button focus behavior is not well defined and inconsistent between browsers. For example, using this example with plain JavaScript neither Firefox nor Safari on OSX fire focus or blur events for buttons. I'm not sure if we normalize this behavior at all yet, but we probably should.

It's also worth noting that you are listening for the blur event on a div, which is not an element that can receive focus by default (Chrome seems to ignore that). You'll notice that you're example doesn't work in Firefox or Safari (OSX) even if you don't set disabled. To resolve that you'd need to set tabIndex on the div which lets it receive focus.

OliverJAsh commented 7 years ago

can you verify what browser you're using?

I'm using Chrome 59 (dev channel).

Button focus behavior is not well defined and inconsistent between browsers. For example, using this example with plain JavaScript neither Firefox nor Safari on OSX fire focus or blur events for buttons. I'm not sure if we normalize this behavior at all yet, but we probably should.

Interesting, I didn't realise this.

It's also worth noting that you are listening for the blur event on a div, which is not an element that can receive focus by default (Chrome seems to ignore that)

The blur event does not bubble up to the div, but can be listened to using event capturing (on the way down as opposed to on the way up). This is why I would expect React—when using Chrome—to follow the behaviour of Chrome's event model.

aweary commented 7 years ago

@OliverJAsh It's a tough problem, the reason Chrome's blur event is being ignored is because we explicitly ignore events while reconciliation is occuring. React has an internal transaction system to make sure that updates are atomic, and when a new transaction is initialized it calls ReactBrowserEventEmitter.setEnabled(false) so we don't get a bunch of events that might be dispatched as React is messing with the DOM.

Since Chrome's blur event is dispatched right after React updates the disabled attribute and before the transaction is closed out it gets ignored. To support this we would have to special case this scenario, and I'm not quite sure yet if there's a way to do that well.

sebmarkbage commented 7 years ago

It seems to me that this is a job for ReactInputSelection. In it, we track the currently focused element before reconciliation and then we restore it after reconciliation. We are unable to restore the focus and it is silently ignore.

If the focusNode operation fails, we should dispatch the blur event ourselves since we know that a blur must have happened as a result of reconciliation in this case.

sebmarkbage commented 7 years ago

I think it is fine to special case this because this is already a special case for focus management and we don't have any other plan for it atm. Other than possibly introducing "pivot points" in the reconciler that avoids touching active trees so that we don't need to do manual focus/selection management. However, even that requires us to special case focus as a pivot point. So it seems pretty inherent that we need something like this special case regardless.

aweary commented 7 years ago

It seems to me that this is a job for ReactInputSelection. In it, we track the currently focused element before reconciliation and then we restore it after reconciliation. We are unable to restore the focus and it is silently ignore.

@sebmarkbage I'm happy to work on adapting ReactInputSelection to something that also supports button focus.

If the focusNode operation fails, we should dispatch the blur event ourselves since we know that a blur must have happened as a result of reconciliation in this case.

The blur event is not consistently dispatched by browsers, so we would have to create a native event in those cases. AFAIK React doesn't do that at the moment (all synthetic events are created with a trusted native event) so is that acceptable? Unless I'm missing where that's already happening

sebmarkbage commented 7 years ago

The issue with the native event object is that they don't always have a representative native event. It can basically be any event type. E.g. you can get a keyup keyboard event in a React onChange event.

I think it's probably fine just to let the native event be null in this case.

prajapati-parth commented 7 years ago

I'm not entirely sure if this would help, but I too wasn't able to get the onBlur fire when a button loses focus, then after looking up a bit it turned out that it need to be focused before getting the blur. So basically this would work

<button
  ref='myButton'
  onClick={() => {this.refs['myButton'].focus()}}
  onBlur={() => {console.log('blured')}}>
  Click
</button>

but this won't

<button
  ref='myButton'
  onBlur={() => {console.log('blured')}}>
  Click
</button>
subhero24 commented 6 years ago

I encouter the same bug when using a textfield that has focus and gets disabled on the next render. A native blur event fires, but the react attached event handler doesn't (using Chrome 64).

Below is a minimal example that shows the bug. It renders a textinput and a button. If you focus the textfield, and then submit by pressing enter, only a native blur event is logged. The react blur event does not fire.

class App extends PureComponent {
    state = {
        disabled: false,
    };

    ref = e => {
        e.addEventListener('blur', this.onNativeBlur);
    };

    onSubmit = e => {
        e.preventDefault();
        this.setState({ disabled: true });
    };

    onReactBlur = e => {
        console.log('React blur');
    };

    onNativeBlur = e => {
        console.log('Native blur');
    };

    render = () => {
        return (
            <form onSubmit={this.onSubmit}>
                <input ref={this.ref} disabled={this.state.disabled} onBlur={this.onReactBlur} />
                <input type="submit" />
            </form>
        );
    };
}
subhero24 commented 6 years ago

I encountered this issue again with an element where the tabIndex is removed in the next render. Here is an example of an element with a tabIndex prop that gets removed when clicked. The onBlur is never fired but the native implementation works.

This native implementation works

<div id="button" tabIndex="0">Focus me, then click me</div>
<script type="text/javascript">
    let button = document.querySelector('#button')
    button.onclick = function () {
        button.removeAttribute('tabIndex')
    }
    button.onblur = function () {
        alert('onBlur handler called')
    }
</script>

while this React component doesn't fire the blur handler

class extends React.PureComponent {
    state = {
        tabIndex: 0,
    };

    onBlur = () => {
        alert("onBlur handler called");
    };

    onClick = () => {
        this.setState({ tabIndex: null });
    };

    render() {
        return (
            <div tabIndex={this.state.tabIndex} onClick={this.onClick} onBlur={this.onBlur}>
                Focus me, then click me
            </div>
        );
    }
}
oliviertassinari commented 5 years ago

I can reproduce the same problem with an <input> element: https://codesandbox.io/s/rough-wave-04260.

bl00mber commented 4 years ago

@oliviertassinari what's the version of React in your case? I checked your sandbox example and blur event has been fired

Screenshot from 2020-04-05

craigkovatch commented 4 years ago

I'm encountering this issue in Chrome 81 + React 16.8.6 -- a focusable <span> element is being re-created (as opposed to re-rendered; bug in other code that I don't own), which causes it to lose focus. The Grid that contains it doesn't receive a React blur event when this happens, which introduces inconsistency into the Grid's view of focus state. Has anyone figured out any workarounds for this? Maybe binding a native focusout event using addEventListener?

Edit: looks like the native blur event is firing, but the corresponding synthetic event is not :(

Eli-Black-Work commented 4 years ago

I'm encountering the same problem as @subhero24 but on React 16.3.1 and Chrome 83. (That is, when my text input is focused and then disabled, it doesn't receive a blur event).

craigkovatch commented 4 years ago

@Bosch-Eli-Black FYI the native event handlers work. It's an unfortunate workaround, but you can use a callback ref to add a handler that works:

function someBlurHandler(e) { ... }
function handleInputRef(ref) {
  if (ref) {
    ref.addEventListener('blur', someBlurHandler);
  }
}
return <input ref={handleInputRef} ... />

No need to remove the event listener in modern browsers, it'll get removed when the node is removed.

eps1lon commented 4 years ago

Some notes on browser inconsistencies: Firefox has an additional bug where the button stays focused even though it becomes disabled. If React want's to normalize blur behavior across browsers it should also actually blur disabled elements.

Firefox not firing blur when an element is removed (and actually blurred) is tracked in https://bugzilla.mozilla.org/show_bug.cgi?id=559561

Generally React could normalize blur events for elements losing focus because that would actually follow some spec:

A user agent MUST dispatch this event when an event target loses focus.

-- https://www.w3.org/TR/uievents/#event-type-blur

It's a bit more problematic if user agents don't move focus. In the end it's up to user agents to decide what elements are focusable and which aren't.

PS:

No need to remove the event listener in modern browsers, it'll get removed when the node is removed.

-- @craigkovatch

You still add new event listeners on every re-render. The ref handler is new on every re-render so React executes it on every render.

craigkovatch commented 3 years ago

@eps1lon

You still add new event listeners on every re-render. The ref handler is new on every re-render so React executes it on every render.

Is that true? The function identities are stable (no inline ref handlers) and the browser de-dupes identical listener adds, so I think it should be fine.

briandipalma commented 2 years ago

Firefox 106 has fixed their bug so now both Chrome and Firefox will raise a native blur event on disable and React will ignore it. My workaround for this issue is to raise the blur event on the function that's being passed in as that's bringing the React component closer to what the browsers are doing:

import { useRef } from "react";
import { UseFocusConfig } from "src/hooks";

/**
 * We need to manually raise a blur event as React ignores browser events while
 * reconciling: https://github.com/facebook/react/issues/9142
 */
export function useBlurDisabledInput(
  disabled: boolean | undefined,
  input: HTMLInputElement | undefined,
  isFocused: boolean,
  focusHandlers: UseFocusConfig
) {
  function handler(e: FocusEvent) {
    let propagationStopped = false;
    const currentTarget = e.currentTarget as EventTarget & Element;
    const relatedTarget = e.relatedTarget as (EventTarget & Element) | null;
    const target = e.target as EventTarget & Element;
    const reactEvent: React.FocusEvent = {
      ...e,
      currentTarget,
      relatedTarget,
      nativeEvent: e,
      target,
      isDefaultPrevented: () => e.defaultPrevented,
      isPropagationStopped: () => propagationStopped,
      stopPropagation() {
        e.stopPropagation();
        propagationStopped = true;
      },
      persist() {},
    };

    input?.removeEventListener("blur", handler);
    focusHandlers?.onBlur?.(reactEvent);
  }
  const handlerRef = useRef(handler);

  if (disabled && input && isFocused) {
    input.removeEventListener("blur", handlerRef.current);
    handlerRef.current = handler;
    input.addEventListener("blur", handler);
  }
}

I can't use a useEffect hook as that seems to run too late to capture the native DOM blur event so this runs inline to a render. I use the ref to prevent registering multiple versions of the handler. I use isFocused (a boolean to track what React thinks of the input focused state) instead of document.activeElement because somehow the input was never document.activeElement - this totally baffles me as I get a blur event when I register a listener. focusHandlers?.onBlur is just our components onBlur callback.

Looking forward to deleting all this once React fixes this bug.

vHeemstra commented 1 year ago

Is there any update on this?

React is still not firing onBlur when a <button> gets disabled. (See example here)

Some observations

React 18.0.0 Google Chrome 115.0.5790.171 Windows 11

After the button is disabled:

Alternative

If React is sticking with the current implementation of its onBlur, for whatever reason, then maybe it is possible to add onDisabled/onEnabled events?