Clean up css in Contextual

https://gist.github.com/3006391#L5799

I added some webkit border radiuses to fix some warnings. Here's the remaining css warnings.

csslint: No errors in contextual.base-rtl.css.
csslint: No errors in contextual.base.css.
csslint: No errors in contextual.theme-rtl.css.
csslint: There are 4 problems in contextual.theme.css.

contextual.theme.css
1: warning at line 22, col 1
Outlines should only be modified using :focus.
.contextual .trigger {

contextual.theme.css
2: warning at line 24, col 3
Using height with border can sometimes make elements larger than you expect.
border: 1px solid transparent;

contextual.theme.css
3: warning at line 24, col 3
Using width with border can sometimes make elements larger than you expect.
border: 1px solid transparent;

contextual.theme.css
4: warning at line 29, col 3
Using width with padding can sometimes make elements larger than you expect.
padding: 0 2px;

#1290506: Remove webkit-specific border radius from CSS

I applied the patch in #2 to confirm that it was working and removed the warnings at csslint.com for all files except contextual.theme.css. I also tried resolving the remaining four warnings but only ended up introducing new issues. I would suggest this is RTBC since the remaining four warnings are not critical errors we're trying to address per the parent ticket (#1190252: Meta: Use csslint as a weapon to beat the crappy CSS out of Drupal core). The specific region these issues are referring to (.contextual .trigger) is limited enough that the box warnings shouldn't be a problem.

see #4. we don't need it for CORE.

There were actually more issues. Most were related to using width/height and border/padding. Using box-sizing actually makes csslint aware that you know what you're doing, so I've ported some of the changes.

There's an issue with ouline: none, though. It is awful for accessbility (see https://github.com/stubbornella/csslint/wiki/Disallow-outline%3Anone), but it is ugly when applied to stuff that already have a border. I've left the outline enabled for the focused unactive trigger, but once it is active I still get rid of it. Couldn't think of a better solution, so this is what we've got right now. Original implementation didn't show any visual cues of focus to the trigger widget.

Here are some screenshots.

oh wrong patch

#11: csslint-contextual-1662968-9.patch queued for re-testing.

CSS patch meets style guidelines, applied patch, patch failed, no longer applies. reroll to head.

rerolled to head

I can't add (or don't want to add) anything else to the current (as of patch #9) visual cues of the contextual widget. I'd like to RTBC this, but it is a reroll of my own patch :/

Wish we could get rid of the vendor prefix for FF, but it looks like there is no unprefixed version yet (http://caniuse.com/css3-boxsizing) so for now the patch applies and works correctly.

csslint, on my machine, warns that webkit also needs the vendor prefix. caniuse.com tells that chrome no longer needs, but iphone/android does. Thus I'm fixing that.

re-running csslint also tells that box-sizing doesn't work on ie6/7 (we no longer care) and that "Outlines shouldn't be hidden unless other visual changes are made." which is already addressed, although csslint is unaware of that.

So I hope no one minds if I add the -webkit prefix and keep this as RTBC since I don't break anything. Really.

CSS changes need manual testing in all browsers and all affected core themes:

1. Test pages where the relevant classes are used without the patch applied.
2. Apply the patch, clear the site and browser caches, and test again. Confirm that there are no changes or regressions.

Post before-and-after screenshots for one browser/theme.

Hey everyone, reworked this patch slightly to reflect the new structure, ran it through Lint and the only errors were the IE ones mentioned above and it complained about the use of !important, but now knowing enough about this, maybe there's a reason for that.

It all looks fine visually to.

Yay, thanks for picking this up! :D However, you shouldn't mark your own patches RTBC, so you can give someone else the chance to just look it over first and make sure it's cool.

Looks like this could use some more eyeballs from some front-end folks. I was curious about the vendor prefixes on those properties, but John/Morten seemed to think they were ok.

+++ b/core/modules/contextual/contextual.theme.cssundefined
@@ -23,18 +23,23 @@
+  border-radius: 4px 4px 0 0;  ¶
...
+  -webkit-box-sizing: border-box; ¶

traling spaces

+++ b/core/modules/contextual/contextual.theme.cssundefined
@@ -23,18 +23,23 @@
+  box-sizing: border-box;

why add box-sizing ?

+++ b/core/modules/contextual/contextual.theme.cssundefined
@@ -23,18 +23,23 @@
+  border-bottom: none;

why add this one.

I think most of this work was addressed in #849926: links wrapped in .contextual-links-wrapper divs are not accessible at all via keyboard alone also problems with screen readers.

Changing status to duplicate - if they were fixed somewhere else that feels more accurate and happier than "won't fix".

I did some analysis just now and there are about 20 warnings in the 4 files in contextual. Not sure if that's worth revisiting or not so I'm not reopening the issue.