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

Button: the disabled attribute doesn't apply to the button element #1775

Closed
connorwyatt opened this issue Jun 16, 2017 · 6 comments · Fixed by #1781
Closed

Button: the disabled attribute doesn't apply to the button element #1775

connorwyatt opened this issue Jun 16, 2017 · 6 comments · Fixed by #1781
Assignees

Comments

@connorwyatt
Copy link

connorwyatt commented Jun 16, 2017

Steps

Create a Button component with disabled set to true.

Expected Result

The button should be disabled.

Actual Result

The button only has styles to make it appear disabled, but it still functions if you tab to it due to a missing disabled attribute on the button element.

Version

0.68.3

Testcase

https://codepen.io/anon/pen/OgbeXr

@layershifter
Copy link
Member

As I see, disabled button doesn't receive focus due negative tabIndex, however HTML's button tag can have a disabled attr. I think, that we need to pass disabled prop if an element is button:

<Button disabled />
<Button as='div' disabled />
<button class="ui disabled button" disabled></button>
<div class="ui disabled button"></div>

@layershifter layershifter changed the title Button does not apply the disabled attribute to the button element Button: the disabled attribute doesn't apply to the button element Jun 16, 2017
@connorwyatt
Copy link
Author

Yes exactly, however the tab index isn't applied when you use it inside a FormButton. Maybe that is a separate issue, but in this case it means that inside a form it allows submission when the button is disabled by tabbing.

@connorwyatt
Copy link
Author

A further issue is that the disabled prop is stripped from the props as it is a handled prop of Button. How do you handle these cases when there is a prop that is both handled, but should also be passed through?

@layershifter
Copy link
Member

I think we can do there something like this:

const disabledButton = disabled && ElementType === 'button'

<ElementType {...rest} className={classes} disabled={disabledButton} onClick={this.handleClick} ref={this.handleRef} tabIndex={tabIndex}>
// ...
</ElementType>

However, it's quite ugly solution because Button's render has already too many conditions. Open for better ideas.

@connorwyatt
Copy link
Author

It seems quite bad to treat the button different to other semantic components. Elsewhere this is handled by splitting the props to get the unhandled props and then spreading them onto the child component. I think the solution should take that into account.

@connorwyatt
Copy link
Author

Thanks a lot for fixing, much appreciated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants