Skip to content

add "focus" to focused btns with button plugin#13907

Merged
mdo merged 1 commit into
masterfrom
fat-12145
Jul 7, 2014
Merged

add "focus" to focused btns with button plugin#13907
mdo merged 1 commit into
masterfrom
fat-12145

Conversation

@fat

@fat fat commented Jun 24, 2014

Copy link
Copy Markdown
Member

possible solution for #12145.

bit tired tho so might have missed something…

similar to @hnrch02 – except uses focus/blur instead of focusin/focusout. I was going to use the latter, but read firefox doesn't support them (https://developer.mozilla.org/en-US/docs/Web/Events/focusout)… and this appears to work, even though i thought they didn't bubble 👀 weird.

went back and forth with toggle class, but ultimately thought it might be safer to tie specific events to specific actions

@cvrebert cvrebert added feature and removed feature labels Jun 24, 2014
Comment thread js/button.js

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why is/was it ^= as opposed to =?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Because of button groups, they toggle via data-toggle="buttons".

@hnrch02

hnrch02 commented Jun 24, 2014

Copy link
Copy Markdown
Collaborator

I guess we can't do much at this time about the disability to focus radio inputs, right?

@fat

fat commented Jun 26, 2014

Copy link
Copy Markdown
Member Author

@hnrch02 huh? don't follow sorry?

@hnrch02

hnrch02 commented Jun 26, 2014

Copy link
Copy Markdown
Collaborator

@fat Try to tab to Radio 2 in this Bin.

@hnrch02

hnrch02 commented Jul 1, 2014

Copy link
Copy Markdown
Collaborator

This needs the styles for .focus, BTW. /cc @mdo

@mdo mdo self-assigned this Jul 2, 2014
@mdo

mdo commented Jul 6, 2014

Copy link
Copy Markdown
Member

@hnrch02 I'll add the styles if necessary once the JS works across the board. That jsbin you linked to still wasn't working for me. I do see a focus state now though, just not on all those radio buttons.

@hnrch02

hnrch02 commented Jul 6, 2014

Copy link
Copy Markdown
Collaborator

@mdo Yeah, that's what I was trying to explain. Only the first radio button can be targeted by tabbing: http://jsbin.com/cubiy/1 Anything else that didn't work for you?

@fat

fat commented Jul 6, 2014

Copy link
Copy Markdown
Member Author

@hnrch02 in your demo: http://jsbin.com/fikul/5/edit

when you tab to the radio, you just use your keyboard left, right, etc.…

@fat

fat commented Jul 6, 2014

Copy link
Copy Markdown
Member Author

i believe that is expected for radio inputs, accessibility, etc. but i could be mistaken

@hnrch02

hnrch02 commented Jul 6, 2014

Copy link
Copy Markdown
Collaborator

Oh wow, I see, wasn't aware of that. I guess then it's good to go 😄

@fat

fat commented Jul 6, 2014

Copy link
Copy Markdown
Member Author

@mdo - does it work for you if you use arrows… just want to confirm before mergin

@mdo

mdo commented Jul 6, 2014

Copy link
Copy Markdown
Member

Ohhhhhh snap. 👍

@mdo

mdo commented Jul 6, 2014

Copy link
Copy Markdown
Member

@hnrch02 What other styles do we need added?

screen shot 2014-07-06 at 1 06 41 am

@hnrch02

hnrch02 commented Jul 6, 2014

Copy link
Copy Markdown
Collaborator

@mdo Styles for the focus class? It's all about the checkboxes and radios.

@mdo

mdo commented Jul 7, 2014

Copy link
Copy Markdown
Member

As far as I can tell they are receiving focus styles.

mdo added a commit that referenced this pull request Jul 7, 2014
add "focus" to focused btns with button plugin
@mdo mdo merged commit 907b3b2 into master Jul 7, 2014
@mdo mdo deleted the fat-12145 branch July 7, 2014 07:37
@cvrebert cvrebert mentioned this pull request Jul 7, 2014
@hnrch02

hnrch02 commented Jul 7, 2014

Copy link
Copy Markdown
Collaborator

@mdo Well, then I must be doing something wrong:
Screenshot

Checkbox 1 has the focus class but no outline.

@mdo

mdo commented Jul 7, 2014

Copy link
Copy Markdown
Member

@hnrch02 I see focus styles in Chrome, Firefox, and Safari on OS X. You on Windows?

@hnrch02

hnrch02 commented Jul 7, 2014

Copy link
Copy Markdown
Collaborator

@hnrch02 Nope, Chrome 36.0.1985.103 beta on OS X 10.9.4. Also not seeing them in Firefox 30. Safari.

@mdo

mdo commented Jul 7, 2014

Copy link
Copy Markdown
Member

Damn beta software. I wonder if Chrome is changing how it handles focus? I see what I posted in my screenshot without issue.

@hnrch02

hnrch02 commented Jul 7, 2014

Copy link
Copy Markdown
Collaborator

@mdo Just to be clear, you tab to Checkbox 1 and you see a focus outline?

mdo added a commit that referenced this pull request Jul 7, 2014
@mdo

mdo commented Jul 7, 2014

Copy link
Copy Markdown
Member

screen shot 2014-07-07 at 12 56 20 am

@hnrch02

hnrch02 commented Jul 7, 2014

Copy link
Copy Markdown
Collaborator

@mdo Would you mind opening the CSS tab in that Bin? It includes a modified version of the button-variant mixin with the focus class.

@mdo

mdo commented Jul 7, 2014

Copy link
Copy Markdown
Member

Lol.

mdo added a commit that referenced this pull request Jul 7, 2014
Follow up to #13907: Add .focus styles for buttons
hnrch02 added a commit to hnrch02/bootstrap that referenced this pull request Jul 16, 2014
fat added a commit that referenced this pull request Jul 17, 2014
Follow-up to #13907: simplify JS logic for focus shim
stempler pushed a commit to stempler/bootstrap that referenced this pull request Nov 4, 2014
krissihall pushed a commit to krissihall/bootstrap that referenced this pull request Nov 26, 2014
* 'master' of https://github.com/krissihall/bootstrap: (210 commits)
  grunt dist
  Use HTTPS in CDN URLs in _config.yml
  Tabs to spaces
  speed up js tests a bit
  Follow-up to twbs#13907: simplify JS logic for focus shim
  regenerate docs/assets/js/docs.min.js
  popover dismiss-on-next-click example: instead of <button>, use <a> w/ tabindex
  Fix jsbin link
  Another new js bin link
  add docs note about browsers w/ JS disabled; fixes twbs#14134
  fix twbs#14114 mo' betta
  Fix inaccessible buttons.
  typos
  Redundant thanks to bb1286a
  grunt
  Fixes twbs#14074: Make open dropdown nav links in navbar use gradients
  Fixes twbs#14133
  Fixes twbs#14132: add .alert-dismissible to docs examples
  Use bootstrapcdn links
  New Android select example
  ...
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.

4 participants