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

fix(accessible-button): use not-allowed cursor on disabled #1114

Merged
merged 2 commits into from
Oct 21, 2019

Conversation

montezume
Copy link
Contributor

Summary

Refactors our Buttons to display cursor: not-allowed when disabled.

Fixes #1113

@montezume montezume requested a review from jonnybel October 21, 2019 09:13
@montezume montezume self-assigned this Oct 21, 2019
expect(button).toHaveAttribute('aria-disabled', 'true');

button.click();
expect(onClick).not.toHaveBeenCalled();
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

before we couldn't test this as pointer-events: none doesn't stop you from calling onClick manually 😆

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

although TBH testing disabled is kind of dumb

Copy link
Member

@emmenko emmenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor

@jonnybel jonnybel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using pointer-events: none for disabled buttons/inputs isn't a good practice, I like it much better with this approach 👌

@montezume montezume merged commit 2bd07f3 into master Oct 21, 2019
@montezume montezume deleted the ml-button-fix branch October 21, 2019 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 Type: Bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Buttons should have not-allowed cursor
3 participants