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 Edge States #87

Merged
merged 5 commits into from
Apr 15, 2015
Merged

Button Edge States #87

merged 5 commits into from
Apr 15, 2015

Conversation

aaronshekey
Copy link

This is a new pull request split from #84, now without the brand-color exploration commits.

A number of button changes here, mostly focused on little edge-case polish.

Changes

Left: Old
Right: New

  1. Focused states are important. I think we should maintain the focused border regardless of hover states.
    focus
  2. Selected buttons should still have a hover state since you can still interact with them.
    selected-hover
  3. Focused buttons within the button groups are now the highest in the z-order as opposed to being overlapped by the button to the right.
    focus-z

Issues

These issues are beyond the scope of Primer, but they should be noted.

  1. By showing the focused state, we're exposing some inconsistent button implementation throughout github.com. Some buttons receive focus when pressed, others not.
  2. By having a hover state on .selected buttons, the dropdown select menus don't receive a hover event because mouse events are inhibited by a blocking overlay div.

@@ -34,6 +34,11 @@
box-shadow: 0 0 5px rgba(81, 167, 232, 0.5);
}

&:focus:hover,
&.selected:focus {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need &:active:focus as well?

@mdo
Copy link
Contributor

mdo commented Mar 31, 2015

By having a hover state on .selected buttons, the dropdown select menus don't receive a hover event because mouse events are inhibited by a blocking overlay div.

Have an example of this?

@aaronshekey
Copy link
Author

By having a hover state on .selected buttons, the dropdown select menus don't receive a hover event because mouse events are inhibited by a blocking overlay div.

select-menu

@mdo
Copy link
Contributor

mdo commented Apr 1, 2015

Ohhhh yeah, the .modal-backdrop or whatever it's called. Makes sense—thanks for the gif!

This'll work into a v2.1 release, so it'll be a few days at least before it's out. Is that cool @aaronshekey or do you need this sooner?

@aaronshekey
Copy link
Author

I'm in no rush on these. My fear is that while these styles are good for a consistent experience across buttons, we don't consistently implement them across github.com. In an effort to make these more proper, we might (in the short term) introduce some jank. Hmm.

@mdo mdo added this to the v2.1.0 milestone Apr 1, 2015
@mdo mdo added the css label Apr 1, 2015
mdo added a commit that referenced this pull request Apr 15, 2015
@mdo mdo merged commit 8eb6a0a into primer:master Apr 15, 2015
@mdo
Copy link
Contributor

mdo commented Apr 15, 2015

<3

@mdo mdo mentioned this pull request Apr 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants