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

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

YesCT’s picture

Issue summary: View changes

embedded images.

markhalliwell’s picture

Status: Needs review » Reviewed & tested by the community

Looks good to me.

webchick’s picture

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.

seanr’s picture

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.

markhalliwell’s picture

^ 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.

markhalliwell’s picture

Assigned: Unassigned » JohnAlbin
Wim Leers’s picture

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.)

alexpott’s picture

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.

alexpott’s picture

Issue summary: View changes

formatted src correctly with actual spaces

Wim Leers’s picture

Title: Contextual links hover state overriden by !important on default state » Bartik 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
FileSize
1.52 KB
44.31 KB
44.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.

markhalliwell’s picture

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++

seanr’s picture

Nice, looks much better.

catch’s picture

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.

Wim Leers’s picture

Issue tags: -sprint
bnjmnm’s picture

Assigned: Unassigned » bnjmnm

I am working on the backport to 7.x

bnjmnm’s picture

Status: Patch (to be ported) » Needs review
FileSize
1.13 KB

I backported the patch to 7.x

bnjmnm’s picture

Assigned: bnjmnm » Unassigned
seanr’s picture

+++ 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.

Wim Leers’s picture

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!

bnjmnm’s picture

Assigned: Unassigned » bnjmnm
Status: Needs work » Needs review
FileSize
1.13 KB

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!

mimes’s picture

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

bnjmnm’s picture

Assigned: bnjmnm » Unassigned
littlepixiez’s picture

Issue summary: View changes
FileSize
15.43 KB
11.33 KB

Currently on Bartik 7.50, the footer's contextual links have underlines.






I've tested this patch on Bartik 7.51-dev and it removes the underline on hover.





This appears to produce the desired result and I can see the changes in the CSS from the patch. Can we close this issue?

  • catch committed c465f96 on 8.3.x
    Issue #2054055 by seanr: Bartik inappropriately styles all links in the...

  • catch committed c465f96 on 8.3.x
    Issue #2054055 by seanr: Bartik inappropriately styles all links in the...

  • catch committed c465f96 on 8.4.x
    Issue #2054055 by seanr: Bartik inappropriately styles all links in the...

  • catch committed c465f96 on 8.4.x
    Issue #2054055 by seanr: Bartik inappropriately styles all links in the...