Background color does not stick when background overlay is used

[christianwach]

@BoweFrankema before I do a pull request, could you please review the menu restyling that I've done on:

http://www.paulschacht.net/

It's my assumption that this is what the menus (both base and main) were supposed to look like, but that some odd CSS declarations crept in somewhere.

My update removes the (too generous) padding, lines up submenus correctly, makes the spans block elements (so the rollover state isn't lost when the page title wraps and you hover between the lines of the title) and extends the links to the full width of the menus and submenus so there aren't any inactive regions. It also leaves the "active trail" items in their highlighted state.

Cheers, Christian

[christianwach]

@BoweFrankema I should have pointed you to the commit too, I guess! 7f85ef6

[christianwach]

I'll keep adding some further notes on issues as I stumble across them...

When the Background Color option is set for a menu (in this case the Below Header Menu), it appears that Infinity writes some CSS into 'dynamic.css'. However, it has no effect, because setting the background-color alone is not enough. The background-image declaration has to be cleared for the background-color to show. The CSS to so this properly would therefore be:

body.theme-option .sub-menu {
  background-color: #ff006f; /* my chosen bg-color */
  background-image: none; /* clear gradient */
}

For Sub-item Backgrounds, things get more complex, because the anchors themselves have their background-image set to:

.base-menu ul li.current-cat a, 
.base-menu ul li.current_page_item a, 
.base-menu ul li.current-menu-item a {
  font-weight: bold;
  background: url('/wp-content/themes/cbox-theme/assets/images/bg-body.gif') repeat transparent;
}

Therefore, the override has no effect as it is targetting the wrong element:

body.theme-option .sub-menu ul ul {
  background-color: #62ff00;
}

I've had a quick look at Infinity_Options_Policy but can't figure out where these CSS overrides are being built. I guess this is one for another time, given that I can adapt the theme with my own CSS, but I'd imagine people who want to use the Theme Options might be frustrated that they have no effect.

[r-a-y]

@christianwach - Also see my related ticket on the Infinity repo:
PressCrew/infinity#83

[r-a-y]

Okay gang, so how do we want to solve this?

My fix in PressCrew/infinity#83 works. The only issue is the menus don't really gel after applying a custom background color.

See the Home button after the background color is altered to red. I guess this is a user CSS issue more than anything.

[christianwach]

@r-a-y As far as I can tell, your fix only works to a degree. I've spent a while on this and I'm fairly sure now that there are deeper issues at play here. The CSS from the "Background Color" and "Sub-item Background" options suffer from the same issue I mention in #198 (comment) in that they generate declarations that are:

• not specific enough to the menu they refer to: for example, if you set a "Background Color" and/or "Sub-item Background" colour on the menu below the header you get colours which "leak" over to sub-menus of the other two menus
• only compatible with one another in certain cases, such as when the background-image is a PNG with alpha transparency: as far as I can tell, opacity won't work with other file types, contrary to Fix background color setting so it applies on the frontend PressCrew/infinity#83 (comment)
• not applied to the anchors in the menu items themselves: thus your "Home" button not being the appropriate colour

"Well if I were you, I wouldn't be starting from here" as the old joke goes...

[r-a-y]

Thanks for looking into this, @christianwach.

I didn't even see the "Sub-item Background" setting. (Too many options!) I have a potential fix for this.

In /cbox-theme/engine/config/features.ini - change line 466 to

style_selector = "#sub-menu-wrap ul li.current-menu-item > a, #sub-menu-wrap ul a:hover"

Needs a bit more work.

This addresses the "Home" button background colour as well. I haven't looked into the background-image issue, but the overlay and background colour settings worked okay for me.

[christianwach]

@r-a-y Nice find - there's definitely progress to be made by tweaking the selectors in that file. I've tried a few variations there, but cannot come up with a solution that works for all combinations of options. Indeed, I'm now confused about the intent of the options... either there are too many or too few to be logical:

"Font Color" and "Font Weight" apply to all items in a menu, while "Background Color" only applies to the menu "bar" (red in your screenshot) and not the background of the dropdowns, which have

background: url('/wp-content/themes/cbox-theme/assets/images/bg-body.gif') repeat transparent;

applied to them in layout.css by default. This, of course, overrides any colour subsequently applied to them. With your fix applied, "Sub-item Background" sets the hover colour of menu items as well as the current item but breaks when you also set "Background Overlay" unless you clear the "Background Color". I could go on, but this rabbit hole goes deep.

I think the "too few" or "too many" options issue is perhaps the key here. At present, people may expect the options to do different things because the options themselves are poorly defined. It seems to me (assuming that the current set of options remains in place) that the menu options should break down thus:

Each menu has base properties such as:

• a general background that should apply to all backgrounds, including dropdown backgrounds
• a general item hover background colour that should apply to all items, including in dropdowns
• a general font colour that should apply to all items, including in dropdowns
• a general hover font colour that should apply to all items, including in dropdowns
• ditto for font weight, though I'm not a fan of this being applied differently on hover since it can cause unexpected wrapping
• other properties as desired (background overlays, borders, padding etc etc)

Each menu then also has the following ancestry properties:

• an item background that should apply to the active item
• a font colour that should apply to the active item
• other active item properties as desired
• an item background that should apply to the active trail
• a font colour that should apply to the active trail
• other active trail properties as desired

This may not be an exhaustive list. My point is that the current set of options do not reflect the actual functioning of the menus and that this leads to the CSS rule confusion. And then there's that pesky .sub-menu naming collision, which complicates things even further...

[MrMaz]

I wouldn't be against dropping these complex options from Infinity completely. It's apparent that either the original/older Infinity design was more compatible with these settings or simply weren't tested thoroughly enough. I would like to completely replace the superfish menu with something else in the future anyways, at which point we can rethink this from the ground up.

Over time with feedback about Infinity coming in, it's become very clear to me that "forcing" end users to learn a bit of CSS if they want to get very specific is quite a bit less painful than trying to cover all cases with a GUI type solution.

I'm not trying to throw your work so far out the window. I'm on board either way. Just wanted to let you guys know that I'm not married to these settings ;)

[christianwach]

@MrMaz I wholeheartedly agree!

[boonebgorges]

Over time with feedback about Infinity coming in, it's become very clear to me that "forcing" end users to learn a bit of CSS if they want to get very specific is quite a bit less painful than trying to cover all cases with a GUI type solution.

Yes :)

@r-a-y @christianwach I've gotten a bit lost trying to parse out the possible solutions presented above. I definitely get the sense that there are a number of options - overlay, color, and each of these for the sub-items - and that actually "fixing" this issue is likely to involve rethinking the level of customizability that we actually want to provide here. But for the purposes of the maintenance release, that kind of rethinking is not viable, so let's instead do one of the following:

1. Call it a "known bug" and say wontfix
2. If there's a way that we can make this even more of an edge case by fixing it for top-level items (while leaving a bug in place for sub-items), let's do it. I get the sense that this is what @r-a-y's fix boils down to.

Thanks!

[r-a-y]

If there's a way that we can make this even more of an edge case by fixing it for top-level items (while leaving a bug in place for sub-items), let's do it.

I went with this option :)

Going to leave this issue open though for the time being.