Modules should provide functionality, themes should style. This is why I am marking this as a bug, themes shouldn't really have to override styles that are thrown in by a module. Take for instance a view, it doesn't provide any styling, just markup. On occasion for things like the Grid style plugin, it will provide some structural CSS, but it doesn't actually throw in any cosmetic styles like border or background colors.

I'll upload a patch soon, just going to take sometime to figure out what it is I actually don't need.

CommentFileSizeAuthor
#3 1836666-facetapi_collapsible-remove_unnecessary_styling-3.patch3.58 KBmarkhalliwell

Comments

markhalliwell’s picture

markhalliwell
Primary language English
commented
Title: Remove Unecessary Styling » Remove Unnecessary Styling

Might help if I could spell today too lol

attiks’s picture

attiks commented

You're right, looking forward to the patch

markhalliwell’s picture

markhalliwell
Primary language English
commented
Status: Active » Needs review
StatusFileSize
new 1836666-facetapi_collapsible-remove_unnecessary_styling-3.patch3.58 KB

Patch attached. Note the block h2 title toggle works with: #1836748-1: Theming hooks break site theme.

attiks’s picture

attiks commented
Status: Needs review » Fixed

I applied the patch but changed it a bit, since on my test site I don't have .content inside the blocks, so switched it to ".facet-collapsible-wrapper"

I also left the icons, so if themers want they can use the images, otherwise they can override the style using, I don't have .block.facetapi-collapsible.expanded.active h2.block-title .toggle on my version

.facetapi-collapsible h2 {
  background: url(images/expand.png) no-repeat right center;
}
.facetapi-collapsible.expanded h2 {
  background: url(images/collapse.png) no-repeat right center;
}

Feel free to re-open if needed and thanks for the patch.

markhalliwell’s picture

markhalliwell
Primary language English
commented

The reason I changed the CSS for applying the toggle images in the block header to .block.facetapi-collapsible.expanded.active h2.block-title .toggle is because themes tend to style h2.block-title. You will have that .toggle class on your version. In #1836748-1: Theming hooks break site theme, I injected <span class="toggle"></span> into the title. This way, by default, it will use FAPIC's images for expand/collapse and give the ability for the theme to alter/hide it if desired.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.