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

Removing tabIndex from component's attributes does not remove tabIndex once set #2528

Closed
mczepiel opened this issue Nov 15, 2014 · 18 comments
Closed

Comments

@mczepiel
Copy link

Once a tab index has been put in place "removing" by the method seen below only ever sets it to -1, which does what I'd expect in that the element isn't focused when tabbing, but it is still focusable by clicking directly on the element.

var attributes = {}
if (this.props.focusable) {
  attributes.tabIndex = 0;
}

return React.createElement("div", attributes, "test");
@zpao
Copy link
Member

zpao commented Dec 1, 2014

Just to clarify… you're explicitly setting it to 0 but React ends up setting it to -1 instead?

cc @syranide

@syranide
Copy link
Contributor

syranide commented Dec 1, 2014

#1510 is the fix (default value is not identical to no attribute). I'll try to review and re-test it wednesday, it fixes quite a lot of edge-cases so it would be good to have it merged...

@mczepiel
Copy link
Author

mczepiel commented Dec 1, 2014

Sorry, with the example code above:

  • Render once with the props.focusable === true
  • Attribute is set as -1 in rendered DOM as expected.
  • Render a second time with props.focusable === false
  • Attribute is now 0 in rendered DOM

I wanted to remove the tabIndex attribute completely in this case, not set it to 0.

Explicitly setting a value works fine but something is assuming that if I had a tabIndex attribute, and then I set no explicit value, it should be 0.

@syranide
Copy link
Contributor

syranide commented Dec 1, 2014

@mczepiel I believe you mixed up the order, anyway... there are a handful of DOM attributes which do not exhibit default behavior when set to the default value, the attribute has to be removed to restore default behavior.

@syranide
Copy link
Contributor

syranide commented Dec 2, 2014

Oh wait, my bad @mczepiel ... yeah there's a second bug here, that you were referring to.

@EvHaus
Copy link

EvHaus commented Feb 12, 2015

I am having a very similar issue with the pattern attributes as well. The code is something like this:

<Input pattern={this.state.hasPattern ? '^((?!@).)*$' : undefined} />

When I disable the hasPattern state, the pattern is set to "undefined" (the string) instead of removing the pattern attribute from the DOM element. This results in a broken input field.

Similarly, I tried this approach. It didn't work either:

var inputProps = {};
if (this.state.hasPattern) inputProps.pattern = '^((?!@).)*$';
<Input {...inputProps}/>

Still -- the pattern attribute is not removed from the DOM element.

@kidwm
Copy link

kidwm commented Feb 24, 2015

I have the same problem with pattern attribute not be removed after DOM updated.

@formula349
Copy link

This is frustrating. I have a

that can have editable cells. Once a <td /> element has a tabIndex prop set to 0 so it can be focused, removing the tabIndex doesn't remove it from the DOM but sets it to -1. This does remove it from the tab order, but still allows focus with the mouse.

Is there any way to remove the tabIndex attribute once it's been set?

@syranide
Copy link
Contributor

@formula349 key can be used for now... and possibly should be anyway in your case (hard to say without more info). Anyway, there's some movement now on #1510.

@formula349
Copy link

Yes, thank you I will use that for now

@loganfuller
Copy link

@syranide I'm running into the same tabIndex issue. What do you mean by "key can be used for now"?

On 0.13.3 the corresponding DOM node for a component which once had the tabIndex prop set but later had it removed will have its tabindex attribute set to -1. Setting a unique key on the component doesn't seem to change anything.

@syranide
Copy link
Contributor

@loganfuller Something like <a tabIndex={tabIndex} key={'foobar:' + tabIndex} />

@loganfuller
Copy link

@syranide Ah, I see, to force the creation of a new DOM element.

That would work, but I integrate with a third-party library on mount that would be expensive to re-initialize any time the associated DOM component is changed.

I can add an extra wrapper with a static key to handle the integration, but wouldn't it be more performant to manually removeAttribute on the DOM element when the component updates rather than forcing the creation of a new one?

@kidwm
Copy link

kidwm commented Sep 24, 2015

@loganfuller You can try https://github.com/Olical/react-faux-dom with that third-party library, maybe it helps.

@syranide
Copy link
Contributor

I can add an extra wrapper with a static key to handle the integration, but wouldn't it be more performant to manually removeAttribute on the DOM element when the component updates rather than forcing the creation of a new one?

That requires a bit more manual work, but that is the best workaround yes.

@danez
Copy link

danez commented Jun 27, 2016

Maybe this changed recently, but I just tried this and it worked perfectly fine. tabindex was either there and set to '0' or in the other case it was not there.

tabIndex={selected ? '0' : null}

@syranide
Copy link
Contributor

syranide commented Jun 27, 2016

Which version of React are you using? Should be fixed in React v15

@gaearon
Copy link
Collaborator

gaearon commented Oct 2, 2017

My understanding is this has been fixed.
If not please raise a new issue.

@gaearon gaearon closed this as completed Oct 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants