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

[EuiButton] Focus outlines are getting default browser color instead of custom in Firefox #5538

Closed
miukimiu opened this issue Jan 13, 2022 · 5 comments

Comments

@miukimiu
Copy link
Contributor

In Firefox 96.0 when we focus on any type of buttons they show the default browser color instead of custom.

Screen.Recording.2022-01-13.at.04.55.54.PM.mp4

In the past, these buttons were getting a square outline matching the current color. As we can see on the following comment:

// The latest theme utilizes `focus-visible` to turn on focus outlines.
// But this is browser-dependend:
// 👉 Safari and Firefox innately respect only showing the outline with keyboard only
// 💔 But they don't allow coloring of the 'auto'/default outline, so contrast is no good in dark mode.
// 👉 For these browsers we use the solid type in order to match with \`currentColor\`.
// 😦 Which does means the outline will be square

// this outline color is not being applied
.euiButton--primary.euiButton--fill:focus {
  outline-color: #000;
}

// firefox gets this styles instead and applies the browser default color
:focus:focus-visible {
  outline-style: auto;
}

Should we improve these styles to target chrome only?

// 👀 Chrome respects :focus-visible and allows coloring the \`auto\` style
&:focus-visible {
outline-style: auto;
}

@cchaos
Copy link
Contributor

cchaos commented Jan 13, 2022

As a reminder, the idea is that there is a consistent experience within a single browser. We do not need to have a consistent experience across browsers. There's known performance issues with plugins that target specific browsers and there's no good way to do that in CSS. How would you propose we only target Chrome with this enhancement?

@miukimiu
Copy link
Contributor Author

miukimiu commented Jan 14, 2022

@cchaos I agree that:

We do not need to have a consistent experience across browsers

But in Firefox the experience used to be better. Right now there is not enough focus ring contrast for some buttons.

It seems that focus-visible started being supported in Firefox 85. This makes this version of FF behave more similarly with Chrome.

But in FF the outline-style: auto; behaves differently than Chrome. Once this style is set, a custom outline color is not applied.

So after some thought, and because the problem is with Firefox. Here are some ideas.

Option 1 ) We can target firefox instead of Chrome (that I agree is much more difficult) and specify the type of outline-style we want:

@-moz-document url-prefix() {
  &:focus-visible {
     outline-style: solid;
  }
}

Option 2) We can just define outline-style: solid; for all browsers that support focus-visible. In Chrome the solid is a little bit different than the auto style. It seems that the auto is applied inside the button and solid is applied outside.

Here are some tests with for outline-style in FF and Chrome:

Outlines FF and Chrome@2x

Let me know if you agree with any of the above two options. If there is something I might be missing. Or if you have a better idea.

@cchaos
Copy link
Contributor

cchaos commented Jan 14, 2022

Ahhh, so I went back to the originating PR: #4242 Where you can see in the screenshot that FF & Safari were getting the right color for the outline, but it seems that the latest versions of these browsers broke that implementation. Sorry, I think I was misunderstanding where the problem was occurring (because I was certain that I had originally accounted for proper coloring/contrast in these other browsers, so I went on the defensive 🤦‍♀️ ).

I think the main reason I forced auto in Chrome is becuase they have a double-border where one color is the color you specify, the other is white which is helpful in dark mode and for ghost buttons without having to add an offset:

image

I do agree, we need to fix Firefox, but I'm wary of both suggestions...

  1. I don't think @-moz-document url-prefix() will be supported for long.
  2. We'd have to re-evaluate all components when switching to solid for all browsers.

But no. 2 is a possibility, I'd just take the time to really test it out.


The one other thing that I'll note here that I worry about is having too large of an offset or require it on all components. The reason being that browser's still haven't prioritized the visibility of this outline when they but up to the edges of a overflow: hidden element. Which is why components like our button group have to override the offset to be inside:

image

@github-actions
Copy link

👋 Hey there. This issue hasn't had any activity for 180 days. We'll automatically close it if that trend continues for another week. If you feel this issue is still valid and needs attention please let us know with a comment.

@github-actions
Copy link

❌ We're automatically closing this issue due to lack of activity. Please comment if you feel this was done in error.

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

No branches or pull requests

2 participants