Skip to content

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

Merged
cvrebert merged 1 commit into
masterfrom
pointer-events-none-a
Mar 26, 2015
Merged

Only disable pointer-events on disabled <a> btns#16092
cvrebert merged 1 commit into
masterfrom
pointer-events-none-a

Conversation

@cvrebert

Copy link
Copy Markdown
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

mdo commented Mar 26, 2015

Copy link
Copy Markdown
Member

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
Copy Markdown

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
Copy Markdown
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
Copy Markdown

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

@lukeman

lukeman commented Jul 1, 2015

Copy link
Copy Markdown
Contributor

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

cvrebert commented Jul 2, 2015

Copy link
Copy Markdown
Collaborator Author

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
Copy Markdown

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

egoroof commented Feb 20, 2016

Copy link
Copy Markdown

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