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

(#289) Remove black outlines on focus #292

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

garlic-os
Copy link

@garlic-os garlic-os commented Sep 23, 2020

Pull Request Description:

By default, most browsers place a black outline around the currently-focused element. On mstacm.org, it looks kind of ugly, so some people want it changed. As a quick fix, this commit completely up removes the black outline around focused elements.
People who use Tab to navigate webpages aren't going to be too happy though, as it's their often only indication of what their cursor is currently on. So I'm hoping we can eventually come up with something else that adds that functionality back in the future.

// Side note: I'm not sure if this is the correct file to put this code in. Please let me know if it isn't.

Checklist:

  • [✓] My code follows the style guidelines of this project
  • [✓] I have performed a self-review of my own code
  • [✓] I have commented my code, particularly in hard-to-understand areas
  • [N/A] I have made corresponding changes to the documentation
  • [✓] My changes generate no new warnings
  • [​X] I have added tests that prove my fix is effective or that my feature works
  • [✓] New and existing unit tests pass locally with my changes
  • [N/A] Any dependent changes have been merged and published in downstream modules

By default, most browsers place a black outline around the currently-focused element. On mstacm.org, it looks kind of ugly, so some people want it changed. As a quick fix, this commit completely up removes the black outline around focused elements.
People who use Tab to navigate  webpages aren't going to be too happy though, as it's their often only indication of what their cursor is currently on. So I'm hoping we can eventually come up with something else that adds that funcionality back in the future.
@garlic-os garlic-os linked an issue Sep 23, 2020 that may be closed by this pull request
@garlic-os
Copy link
Author

I realize now I forgot to add /hotfix to the beginning of that branch name. Sorry about that.

@ClayMav
Copy link
Contributor

ClayMav commented Oct 5, 2020

To solve the accessiblity issue, you could actually remove the stuff youve changed here and merge this functionality with our current selection code. We use :selected to apply styles for button down and what not, if you change the css tags everywhere we have :selected to also change :focus, you can add an outline: none; to those css blocks and that will make it so that as long as we coded a selected behavior for the tag, it will look good for people using tab to navigate. Also don't forget to request a review from someone when you submit a pr!

Copy link
Contributor

@ClayMav ClayMav left a comment

Choose a reason for hiding this comment

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

as above

@ClayMav ClayMav requested review from KerimD and cjwagn1 December 15, 2020 01:28
@KerimD
Copy link
Contributor

KerimD commented Dec 17, 2020

For accessibility purposes you cannot remove those outlines, but you can change them to make them look more appealing.

Copy link
Contributor

@cjwagn1 cjwagn1 left a comment

Choose a reason for hiding this comment

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

Instead of changing everywhere, some of these elements look worse than others. If you want to selectively change the styles that overlap with other styles that would also work.

Example:
image

Or you could change these guys to be a bit more appearing that just a little black border.

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.

Outlines on buttons and textfields
4 participants