Updated: Comment #8

Problem/Motivation

Contextual links look messed up in Bartik's footer.

Proposed resolution

Make them work fine by fixing Bartik's inappropriate behavior to override the styling of *every* link in the footer. (And the font size, too.)

Before
After

Remaining tasks

None.

User interface changes

Contextual links finally won't look like shit anymore in Bartik's footer!

API changes

None.

None.

Original report by @username

See the attached before and after images. This patch assumes that the !important is in fact necessary, so it adds it for the hover state as well. The alternative would be to remove from both. I am not sure what condition would necessitate the !important anyway unless a themer improperly chose their selectors.

before

https://drupal.org/files/Screen Shot 2013-07-30 at 4.11.49 PM.png

after

https://drupal.org/files/Screen Shot 2013-07-30 at 4.12.05 PM.png

Files: 
CommentFileSizeAuthor
#18 bartik_footer_links_breaks_contextual-2054055-18.patch1.13 KBbnjmnm
PASSED: [[SimpleTest]]: [MySQL] 40,464 pass(es).
[ View ]
#14 bartik_footer_links_breaks_contextual-2054055-13.patch1.13 KBbnjmnm
PASSED: [[SimpleTest]]: [MySQL] 40,425 pass(es).
[ View ]
#8 after.png44.52 KBWim Leers
#8 before.png44.31 KBWim Leers
#8 bartik_footer_links_breaks_contextual-2054055-8.patch1.52 KBWim Leers
PASSED: [[SimpleTest]]: [MySQL] 60,235 pass(es).
[ View ]
drupal-contextual-link-hover.patch645 bytesseanr
PASSED: [[SimpleTest]]: [MySQL] 57,291 pass(es).
[ View ]
Screen Shot 2013-07-30 at 4.12.05 PM.png5.61 KBseanr
Screen Shot 2013-07-30 at 4.11.49 PM.png5.68 KBseanr

Comments

Issue summary:View changes

embedded images.

Status:Needs review» Reviewed & tested by the community

Looks good to me.

Status:Reviewed & tested by the community» Needs review

Would love at least one more eyeball from someone with CSS chops. !important always gives me the willies.

webchick, I can see why it was done, though, since it is very likely that a contributed (or even core) theme could accidentally override the link color (even just #footer a {color:white;} would do it). In fact, Bartik's style.css at line 955 would do it. There's not a good way to make contextual specific enough to avoid that since we can't use IDs for all of the contextual elements.

^ is a themer and has "CSS chops". I agree with @seanr's statement in #3. If we "need" to override it, all we have to do is do another !important, but I have never styled contextual links.

Assigned:Unassigned» JohnAlbin

Status:Needs review» Reviewed & tested by the community
Issue tags:+CSS

I wish there were a way to solve this without !important, but then it probably also wouldn't be in the preceding selector. So, RTBC.

(I tested this to verify it's still working.)

Status:Reviewed & tested by the community» Needs work

This feels very wrong to me. I think we should remove all the !important and fix what was reported in the original issue #849926-66: links wrapped in .contextual-links-wrapper divs are not accessible at all via keyboard alone also problems with screen readers

I noticed that the color of the contextual links was being set to white in the Bartik footer. I altered the CSS to prevent alteration to the contextual links color.

Issue summary:View changes

formatted src correctly with actual spaces

Title:Contextual links hover state overriden by !important on default stateBartik inappropriately styles all links in the footer, rather than links inside blocks in the footer, breaking contextual links in the process
Component:contextual.module» Bartik theme
Assigned:JohnAlbin» jessebeach
Priority:Normal» Major
Issue summary:View changes
Status:Needs work» Needs review
Issue tags:+Spark, +sprint
StatusFileSize
new1.52 KB
PASSED: [[SimpleTest]]: [MySQL] 60,235 pass(es).
[ View ]
new44.31 KB
new44.52 KB

The problem is Bartik's #footer-wrapper a { color: something } statement, which has a higher specificity than .contextual-region .contextual .contextual-links a { color: something } because of the ID.

Conclusion: Bartik should not use an ID, or at the very least, it should not target an extremely generic element such as a in a very generic way (i.e. #footer-wrapper: for everything in the footer).

So the solution is to s/#footer-wrapper/#footer-wrapper .block .content/, which is what it's actually targeting anyway.

This in fact also solves a problem that has existed for years: Bartik's footer styling messes up all contextual links in the footer, and in many more ways than only on hover. See for yourself:

Before
After

Assigning to Jesse Beach for review.

Assigned:jessebeach» Unassigned
Status:Needs review» Reviewed & tested by the community

Thanks @Wim Leers! This is certainly a much more preferable approach. I just didn't take the time to dig too deeply and discover the cause of this.
No !important++

Nice, looks much better.

Version:8.x-dev» 7.x-dev
Status:Reviewed & tested by the community» Patch (to be ported)

Committed/pushed to 8.x, thanks!

Moving to 7.x for backport.

Issue tags:-sprint

Assigned:Unassigned» bnjmnm

I am working on the backport to 7.x

Status:Patch (to be ported)» Needs review
StatusFileSize
new1.13 KB
PASSED: [[SimpleTest]]: [MySQL] 40,425 pass(es).
[ View ]

I backported the patch to 7.x

Assigned:bnjmnm» Unassigned

+++ b/themes/bartik/css/style.css
@@ -834,17 +834,17 @@ ul.links {
+#footer-wrapper .block .content{

With the exception of the missing space before "{" I think this is good.

Status:Needs review» Needs work

I agree with #16: bnjmnm's backport is good to go, besides that missing space. No regressions in manual testing. Marking "needs work" for that reroll :) Thanks, bnjmnm!

Assigned:Unassigned» bnjmnm
Status:Needs work» Needs review
StatusFileSize
new1.13 KB
PASSED: [[SimpleTest]]: [MySQL] 40,464 pass(es).
[ View ]

Corrected css spacing from patch originally submitted in comment 13 - ready for review + thanks to everyone helping me along as I get accustomed to the core contribution process!

I can't seem to see the changes made by this patch.