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 styling is slightly and inconsistently broken #1341

Closed
laserlemon opened this issue Apr 13, 2021 · 3 comments
Closed

Button styling is slightly and inconsistently broken #1341

laserlemon opened this issue Apr 13, 2021 · 3 comments
Assignees

Comments

@laserlemon
Copy link

I put together a test page that uses Primer view components to render several variations on the Primer::ButtonComponent, using all three supported tags (button, a, and summary) as well as both disabled and enabled states. For button tags, I used the disabled attribute and for a/summary tags, I used aria-disabled.

From what I can tell, there are five issues:

  1. Disabled, default scheme buttons (for all tags) show a slight shadow as if the button is pushable. This shadow is absent from outline and danger scheme buttons.
  2. Disabled, outline scheme buttons (for button and a tags) change the icon color on hover. The summary tag button does not. I'm not sure what the intended behavior here is but there's at least a consistency problem.
  3. Disabled, danger scheme buttons (for button and a tags) change the icon color on hover. The summary tag button does not. There's the same consistency problem described in number 3 and the additional problem of the icon color changing to white on hover which has sort of a disappearing effect.
  4. Disabled, primary scheme buttons (for button and a tags) show a slight shadow as if the button is pushable. This shadow is (correctly) absent from outline and danger scheme buttons.
  5. Disabled, primary scheme buttons (for all tags) show a gray icon, which looks out of place. Should the icon preserve the white styling of the button text?

Numbers 2, 3, and 5 are also problematic in dark and dimmed themes. Demo included for the dimmed theme.

Demos

Light theme, hovers

2021-04-13 10 24 49

Light theme, clicks

2021-04-13 10 28 45

Dimmed theme, hovers and clicks

2021-04-13 10 49 44

Context

  • OS: macOS Catalina
  • Browser: Safari
  • Version: 14.0.3 15610.4.3.1.7, 15610

I also experience these same behaviors on Firefox 87.0 and Chrome 89.0.4389.114.

@simurai
Copy link
Contributor

simurai commented Jul 27, 2021

The pressed (:active) state of btn-primary seems to have a gray border in "Dark high contrast":

button

Also @Juliusschaeper noticed that the pressed (:active) state of btn-primary has a gray background, but only in Safari. I guess Chrome also uses the :focus style so it is still green? 🤔

6AC688DF-FC18-410F-9283-EB268170F695_1_105_c

@mperrotti
Copy link
Contributor

mperrotti commented Aug 25, 2021

@simurai - I was looking at the issues you posted about while working on https://github.com/github/primer/issues/263

I can't reproduce the Safari issue, but the gray border seems to be happening on all dark modes. It looks like we just forgot to specify colors.btn.primary.activeBorder: https://github.com/primer/primitives/blob/main/data/colors_v2/vars/component_dark.ts#L78

I can submit a PR to fix that as part of my work on that issue.

Edit: Instead of adding a activeBorder property to colors.btn.primary, I'm just going to set the border-color in Primer CSS.

@yaili
Copy link
Member

yaili commented Nov 9, 2021

I transferred this issue to https://github.com/github/primer/issues/468 - closing as duplicate.

@yaili yaili closed this as completed Nov 9, 2021
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

No branches or pull requests

4 participants