Closed (fixed)
Project:
Drupal core
Version:
8.0.x-dev
Component:
Seven theme
Priority:
Normal
Category:
Bug report
Assigned:
Unassigned
Reporter:
Created:
25 Apr 2014 at 15:36 UTC
Updated:
6 Oct 2014 at 11:30 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
mgiffordComment #2
mgiffordComment #3
lewisnymanThanks! In some cases we have removed the outline when we've replaced it with other focus styling.
I was chatting to Bojhan. It would be nice if we could replace the default browser focus styling with our own completely, see the focus styling for the buttons:
Do you have any advice? The styling uses box-shadow which is probably safe seeing as we don't support IE8.
Comment #4
mgiffordAs long as the outline is replaced, it's fine.
Would be good to keep the outline by default in Stark and ensure that the browser+ styling all happens in Seven & Bartik.
I like the new styling on the buttons. I don't see any problem with that. I like that it gives a bit of extra bling with focus vs hover but that otherwise they are the same.
Comment #5
lewisnymanHere's a proof of concept patch that adds default focus styling to everything and removes any instances of outline: 0;
Not sure if I'm a fan of this, because any element that defines it's own focus styling (see the add to shortcuts icons) know has to know to remove box-shadow instead of outline. It also just doesn't look good... I've talked myself out of it :)
Comment #6
mgiffordIf focus is defined every time outline is removed that's fine. It's a big accessibility problem if outline is removed without providing a replacement.
Comment #7
lewisnymanLooks like csslint already has a rule for this: https://github.com/CSSLint/csslint/wiki/Disallow-outline%3Anone
Comment #8
mgifford@LewisNyman This sounds great. I assume you've got it set up locally.
I was expecting to see an Installation section in https://github.com/CSSLint/csslint/wiki
Comment #9
lewisnyman@mgifford I have it running inside of Sublime. It great to see hints as you're typing.
You can set it up any number of ways. They should all be able to read the .csslintrc file for settings so it means we can flexible with everyones preferred method.
Comment #10
mgiffordTrivial spacing issues in formatting. No functional difference from previous patch. Just spacing.
Comment #11
davidhernandezI tested this manually. I'm getting the blue gradient on focus everywhere, not just "buttons". Is that the intended result? I'm also not getting it on the Save button. Without the patch focus underlines the text in the save button and makes the blue a lighter color. With the patch the underline is gone, but there is no blue focus gradient, only the color change.
Comment #12
lewisnyman@davidhernandez Thanks for testing. This glow looks bad on links, I think underlining is enough for those situations. Maybe we should try this glow on the other focusable elements apart from links?
Comment #13
mgiffordComment #14
mgiffordComment #17
scresante commentedRerolled patch
Comment #18
mgiffordfixing the focus problem mentioned in #11.
Comment #19
lewisnymanWhy are we changing the colour to #000? We use #333 everywhere else
This affects all text inputs without focus? I think this styling is supposed to go on focus right?
Comment #20
mgiffordYa... I can't explain that.. Maybe just lingering patch fragments.. I removed all changes from core/themes/seven/css/style.css as none of them are specific to the focus.
They look like I introduced those in #10... Sorry.
Comment #21
lewisnymanSorry! This was in the last patch but now it looks like we have .button:focus declared in two places?
Comment #22
emma.mariaI removed the focus styles from seven.base.css as they were being overridden by the other Seven stylesheets and therefore not needed.
Comment #23
mgiffordWhy are we moving the position of
.button:hover?Comment #24
ByronNorris commentedAttached are screenshots of the various button states after applying the patch in #22.
Comment #27
mgiffordComment #28
pushpinderchauhan commentedReolled #22 patch.
Comment #29
lewisnymanThanks, I think it's traditional to have the focus after the select just incase it needs to override any hover styling.
Thanks for the screenshots @bluegriff!
Comment #32
alexpottCommitted 60441ca and pushed to 8.0.x. Thanks!