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

feat(chore): optional outline on focus #2801

Draft
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

lubber-de
Copy link
Member

Description

This PR allows to declare a specific focus outline for some components where the default outline is set to none.
The default will be kept to none because FUI mostly shows focus by providing a different color (e.g. buttons) or would not visually fit.

To reduce CSS selectors only a value != none will be rendered, so by default the CSS output wont change

Now the user can decide to change @focusOutline globally in a custom site.variables or individually inside a custom themes {component.variables}

I still used the :focus pseudo class instead of :focus-visible as we still support IE11 and and even Safari does only support that since 15.4 and we support the last 4 Safari versions, and we always used :focus everywhere else.

Closes

#2799

@lubber-de lubber-de added type/feat Any feature requests or improvements lang/css Anything involving CSS state/awaiting-reviews Pull requests which are waiting for reviews labels May 24, 2023
@lubber-de lubber-de added this to the 2.9.3 milestone May 24, 2023
@mvorisek
Copy link
Contributor

See https://stackoverflow.com/questions/31402576/enable-focus-only-on-keyboard-use-or-tab-press for analysis how big players style it.

In #2799 I have described an issue for tab focus for button (and dropdown). As the PR is coded now, it will imply outline for :focus, ie. bold outline for buttons clicked by a mouse. This is not wanted.

Is there any way how to simply not add outline: none when :focus-visible is active? At least for browsers that support this pseudo class - https://caniuse.com/css-focus-visible.

@lubber-de lubber-de modified the milestones: 2.9.3, 2.9.x May 24, 2023
@lubber-de
Copy link
Member Author

Is there any way how to simply not add outline: none when :focus-visible is active? At least for browsers that support this pseudo class - caniuse.com/css-focus-visible.

Sure

.ui.button:focus-visible {
  outline: revert;
}

https://jsfiddle.net/lubber/y0ba824e/10/

@mvorisek
Copy link
Contributor

Yes, please add this revert style for buttons and other important components.

@lubber-de
Copy link
Member Author

lubber-de commented May 24, 2023

Yes, please add this revert style for buttons and other important components.

No, are least not now, because that won't fit the FUI default theme and every browser displays that differently. If you want this consistent people should be able to define the outline as desired via changing the less variables in this PR.

And there are combinations like labeled input where this would need additional styling to look good.

If you don't care about all this, then i suggest to add this as custom css in your app. Putting this hardcoded into FUI ,i can already hear several people scream about the visual change in the next version

@mvorisek
Copy link
Contributor

I consider this a bug. .ui.button:focus-visible selector will not focus visually on a button click nor if a button has no tabindex set. So there should be no visual change on anything except when a button is navigated via tab which deserves a proper styling. I belive the current Fomantic-UI simply does not account for tab focus styling.

One proof - a:focus-visible is styled currently natively as well. Just try it. Another proof - for ex. Google does not style it as well ;-).

So I propose:
a) revert the outline for button with .ui.button:focus-visible selector
b) fix tab visual focus of dropdown to match regular (styled by Fomantic-UI) input style (currently dropdown glow is based on open/close, not on (regular) focus)
c) review other outline: none/0 cases case by case

@lubber-de lubber-de added state/on-hold Issues and pull requests which are on hold for any reason and removed state/awaiting-reviews Pull requests which are waiting for reviews labels May 24, 2023
@lubber-de lubber-de marked this pull request as draft May 24, 2023 16:42
@mvorisek
Copy link
Contributor

mvorisek commented May 25, 2023

Here is a default FF style obtained from about:config FF:
image

preview:
image

so outline-offset (and maybe other CSS properties) needs to be reverted as well

Current Fomantic-UI button/dropdown/checkbox UX can be tested on https://dev.atk4.org/demos/form-control/input2.php.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lang/css Anything involving CSS state/on-hold Issues and pull requests which are on hold for any reason type/feat Any feature requests or improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants