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

Only disable pointer-events on disabled <a> btns #16092

Merged
merged 1 commit into from
Mar 26, 2015

Conversation

cvrebert
Copy link
Collaborator

Fixes #16088.
Applying pointer-events: none to non-<a> buttons is unnecessary and prevents their [disabled]:hover styles, in particular their cursor, from applying.

<a> doesn't support the `[disabled]` attribute,
so `a.btn.disabled` simulates it using `pointer-events: none`.
However, this is unnecessary for <button>s and <input>s, and
also prevents their `[disabled]` cursor from displaying.

[skip sauce]
[skip validator]
@cvrebert cvrebert added the css label Mar 17, 2015
@cvrebert cvrebert added this to the v3.3.5 milestone Mar 17, 2015
@mdo
Copy link
Member

mdo commented Mar 26, 2015

Sounds reasonable to me I suppose.

cvrebert added a commit that referenced this pull request Mar 26, 2015
Only disable pointer-events on disabled <a> btns
@cvrebert cvrebert merged commit 02f9f2b into master Mar 26, 2015
@cvrebert cvrebert deleted the pointer-events-none-a branch March 26, 2015 04:58
@cvrebert cvrebert mentioned this pull request Mar 26, 2015
@steph643
Copy link

I think pointer-events: none is here so that you cannot click on the button when it is disabled. If you remove this style, disabled buttons will look like they are disabled but will be in fact enabled (you can click on them and trigger an event).

@cvrebert
Copy link
Collaborator Author

@steph643 We basically don't recommend using the .disabled class on <button>s if possible; use the disabled attribute instead (which itself prevents clicks). <a> doesn't support the disabled attribute (at least not yet), so we use pointer-events: none there to imperfectly approximate the prevention of clicks.

@steph643
Copy link

@cvrebert, thanks a million. I had this mistake in several places in my code.

@lukeman
Copy link
Contributor

lukeman commented Jul 1, 2015

In our local builds I've chosen to add this rule back until we've been able to audit our existing codebase as we've been relying on the effect of .disabled across all .btn-able elements.

I understand the reason for this specific issue, but this type of breaking change seems awfully severe for a bugfix release—whereas .btn.disabled worked consistently before in actually disabling click events across all .btn elements with or without the disabled attribute, this means updating existing code to also set the disabled attribute on buttons.

Given the benefit of this backwards-incompatibility seems limited to styling on mouse hover events, I'd argue that this change would be suitable for a major release (4.0) and not for a bugfix or even minor version bump where compatibility is to be preserved.

@cvrebert
Copy link
Collaborator Author

cvrebert commented Jul 2, 2015

FWIW, our docs only show/mention usage of .disabled on <a>s. I checked pretty-old v3.0.2 and couldn't find any instances of class="disabled" on <button>s in those docs either.

I will see about adding a linter for button.btn.disabled to Bootlint to help folks avoid this.
It might also be worth adding an explicit warning about this to the docs.

1000hz added a commit to 1000hz/bootstrap-validator that referenced this pull request Jul 14, 2015
@jeremyml
Copy link

Is it safe to override the pointer-events: none with

 .btn[disabled] {pointer-events: auto;}

or will I get unexpected behavior? I rather like having the cursor: not-allowed on my disabled buttons.

@egoroof
Copy link

egoroof commented Feb 20, 2016

It's a breaking change! Why was it merged to v3?? Now I have to rewrite some code :(
Thanks @cvrebert for explanation how to fix.

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

Successfully merging this pull request may close these issues.

cursor: not-allowed isn't working on disabled buttons
7 participants