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

autofocus attr not included in button element when rendered #11851

Closed
phallguy opened this issue Dec 14, 2017 · 8 comments
Closed

autofocus attr not included in button element when rendered #11851

phallguy opened this issue Dec 14, 2017 · 8 comments

Comments

@phallguy
Copy link

@phallguy phallguy commented Dec 14, 2017

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

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Dec 14, 2017

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.

@gaearon gaearon closed this Dec 14, 2017
@phallguy

This comment has been minimized.

Copy link
Author

@phallguy phallguy commented Dec 14, 2017

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

@gaearon

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Dec 14, 2017

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

This comment has been minimized.

Copy link
Author

@phallguy phallguy commented Dec 14, 2017

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

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Dec 14, 2017

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

This comment has been minimized.

Copy link
Member

@gaearon gaearon commented Dec 14, 2017

See #11159 (comment) and #11159 (comment) from my last investigation into this.

@jankalfus

This comment has been minimized.

Copy link

@jankalfus jankalfus commented Apr 30, 2018

@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

This comment has been minimized.

Copy link

@MiroslavPetrik MiroslavPetrik commented Jul 25, 2018

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

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