facebook / react

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

autofocus attr not included in button element when rendered #11851

Closed phallguy closed 6 years ago

phallguy commented 6 years ago

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

bug

What is the current behavior?

autoFocus prop on <button /> component is not rendered to the DOM. According to MDN autofocus is a valid attr for the button element.

https://developer.mozilla.org/en-US/docs/Web/HTML/Element/button

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 for React 16: https://jsfiddle.net/Luktwrdm/, template for React 15: https://jsfiddle.net/hmbg7e9w/).

https://jsfiddle.net/pnkso56k/1/

What is the expected behavior?

autofocus attr should be rendered to DOM when provided as a prop to button component.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

React 16.2 in Chrome.

gaearon commented 6 years ago

This is intentional because it works very differently in different browsers. So instead of actually setting a DOM attribute, we polyfill its behavior in JavaScript.

phallguy commented 6 years ago

Oh wasn't aware of the cross browser behavior. What polly fill would you recommend to support native autofocus on buttons?

gaearon commented 6 years ago

We already include the polyfill. When you give autoFocus to React, it calls focus() on initial mount.

I'm not really sure I understand your example. Two things can't be focused at the same time. React focuses the input in your example, but if you remove the input, it will focus the button, just like I'd expect.

phallguy commented 6 years ago

Ahh I see. The example was to demonstrate the presence of the autofocus attr itself on the rendered DOM element. In the real world example that started my research, I was rendering a button element into a dialog and it was not focusing when it was rendered. However using an input element instead in the same dialog would become focused.

I'm trying to make sure that I can have a button in a dialog focused by default for an improved user experience and I wanted to rely on the browser implementation if possible which is often better at handling accessibility concerns.

If the intent in React is to call focus in the same way that the browser would I'll see if I can prepare a more comprehensive example with an appearing component that doesn't receive focus properly.

gaearon commented 6 years ago

React just calls focus(). The browsers are inconsistent about focusing with autoFocus; I'd be very surprised if their implementation is somehow better for a11y. Happy to be proven wrong though.

gaearon commented 6 years ago

See https://github.com/facebook/react/issues/11159#issuecomment-335824907 and https://github.com/facebook/react/issues/11159#issuecomment-335836758 from my last investigation into this.

jankalfus commented 6 years ago

@gaearon The problem with this approach is that it doesn't work if the input element is not visible on mount. I ran into an issue when using Bootstrap 4 modal - I want to focus the first child element that has the autofocus attribute after the modal gets displayed.

I created a generic component that wraps Bootstrap modal:

export default class Modal extends React.Component {
  componentDidMount() {
    $(this.dialog).on("shown.bs.modal", function() {
      $(this)
        .find("[autofocus]")
        .focus();
    });
    $(this.dialog).on("hidden.bs.modal", this.props.onCancelClick);
    $(this.dialog).modal("show");
  }

  render() {
    const { title, confirmText, onConfirmClick, children } = this.props;
    return (
      <div
        className="modal fade"
        tabIndex="-1"
        role="dialog"
        ref={d => {
          this.dialog = d;
        }}
      >
        <div className="modal-dialog" role="document">
          <div className="modal-content">
            {/* removed header markup for clarity */}
            <div className="modal-body">{children}</div>
            {/* removed buttons markup for clarity */}
          </div>
        </div>
      </div>
    );
  }
}

Example of use:

<Modal title="Some modal" onCancelClick={onClose} onConfirmClick={() => {}}>
  <form noValidate>
    <div className="form-group">
    <label htmlFor="email">Email address</label>
    <input
      type="email"
      placeholder="Email"
      value={this.state.email}
      onChange={this._handleEmailChange}
      disabled={disabled}
      name="email"
      id="email"
      autofocus="true"
    />
    </div>
    {/* Removed rest of markup for clarity */}
  </form>
</Modal>

As you can see, I ended up using the DOM autofocus attribute on the input field, rather than autoFocus - because that doesn't work. The problem with this is I'm getting Warning: Invalid DOM property autofocus. Did you mean autoFocus? warning, which is annoying. It would be nice if I could at least disable the warning somehow. I tried searching for a way do disable it but I didn't find anything. There might also be a better way to solve this that I'm not aware of :)

MiroslavPetrik commented 6 years ago

This complicates testing -- I want to assert specific input to be focused. Now I can not just assert hasAttribute but must workaround it.