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

Improve the fallback example for :focus-visible #19993

Merged
merged 7 commits into from
Aug 28, 2022
Merged

Improve the fallback example for :focus-visible #19993

merged 7 commits into from
Aug 28, 2022

Conversation

firefoxic
Copy link
Contributor

@firefoxic firefoxic commented Aug 26, 2022

Summary

Improved code example for fallback on :focus and updated section text to match new code.

Motivation

After replacing the custom button with a native button in the previous change to this example, the styles of the example now affect the native styles of the button. It is better to move the properties that change the appearance of the button itself into its general styles.

In addition, this example has always been difficult to understand for beginners due to different styles for different focus pseudo-classes. Making the example look more realistic and practical will make it more useful for developers.

Supporting details

@simevidas suggested the @supports not selector() method and noticed that setting/resetting background is causing the button to lose the native appearance on click.

And @pepelsbey suggested not styling :focus at all, so older browsers would just have a native focus outline.

This code works well in newer browsers and fallbacks to :focus in older ones as expected. Checked in Safari 14.1:

Kooha-2022-08-24-19-42-56.mp4

Metadata

  • Rewrites (or significantly expands) a document

@firefoxic firefoxic requested a review from a team as a code owner August 26, 2022 09:18
@firefoxic firefoxic requested review from estelle and removed request for a team August 26, 2022 09:18
@github-actions github-actions bot added the Content:CSS Cascading Style Sheets docs label Aug 26, 2022
@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2022

Preview URLs

Flaws

None! 🎉

External URLs

URL: /en-US/docs/Web/CSS/:focus-visible
Title: :focus-visible
on GitHub

No new external URLs

(this comment was updated 2022-08-28 12:45:48.545648)

@firefoxic
Copy link
Contributor Author

Commits are added very quickly to the main branch 🚀
I had to do a rebase 👀

@simevidas
Copy link

I would not change the paragraph above the code example.

A custom control, such as a custom element button, can use :focus-visible to selectively apply a focus indicator only on keyboard-focus. This matches the native focus behavior for controls like .

The :focus fallback is sufficiently explained via the comments in the code example, I think.

Also, since this example is about custom buttons, I would style the background and border of the button to remove the button’s native appearance.

@firefoxic
Copy link
Contributor Author

I would not change the paragraph above the code example.

It seems to me that he lost touch with the code in the last pull request that changes this example, because this text is not about stylized native buttons, but about buttons created using custom elements.

But perhaps the repetition is redundant. I think we can somehow shorten the text.

After replacing the custom button with a native button in the previous change to this example, the styles of the example now affect the native styles of the button. It is better to move the properties that change the appearance of the button itself into its general styles.

In addition, this example has always been difficult to understand for beginners due to different styles for different focus pseudo-classes. Making the example look more realistic and practical will make it more useful for developers.
@simevidas
Copy link

Maybe the title of the section should be changed to something like ”Providing a :focus fallback”.

files/en-us/web/css/_colon_focus-visible/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/_colon_focus-visible/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/_colon_focus-visible/index.md Outdated Show resolved Hide resolved
files/en-us/web/css/_colon_focus-visible/index.md Outdated Show resolved Hide resolved
firefoxic and others added 5 commits August 28, 2022 15:20
Co-authored-by: Jean-Yves Perrier <[email protected]>
Co-authored-by: Jean-Yves Perrier <[email protected]>
Co-authored-by: Jean-Yves Perrier <[email protected]>
Co-authored-by: Jean-Yves Perrier <[email protected]>
@teoli2003 teoli2003 merged commit 7572230 into mdn:main Aug 28, 2022
@firefoxic
Copy link
Contributor Author

@teoli2003 thanks 🙂

@firefoxic firefoxic deleted the focus-visible branch August 28, 2022 12:56
goshdarnheck pushed a commit to goshdarnheck/content that referenced this pull request Sep 7, 2022
* Improve the fallback example for :focus-visible

After replacing the custom button with a native button in the previous change to this example, the styles of the example now affect the native styles of the button. It is better to move the properties that change the appearance of the button itself into its general styles.

In addition, this example has always been difficult to understand for beginners due to different styles for different focus pseudo-classes. Making the example look more realistic and practical will make it more useful for developers.

* Update the section heading

* Improve text

Co-authored-by: Jean-Yves Perrier <[email protected]>

* Improve code comment

Co-authored-by: Jean-Yves Perrier <[email protected]>

* Improve fallback comment

Co-authored-by: Jean-Yves Perrier <[email protected]>

* Remove unwanted part

Co-authored-by: Jean-Yves Perrier <[email protected]>

* Fix test failure (sorry)

Co-authored-by: Jean-Yves Perrier <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Content:CSS Cascading Style Sheets docs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants