Support fancy styling of modules

[cash]

It's currently optional but that really limits themers (or forces them to override the module view)

[Cash Costello]

Fixes #4003 footer in modules is included by default

Changeset: 5879ebf600db1609b0cd5112206fd1e34b722144

[ewinslow]

Can you be more specific about how this limits themers? I thought it made life easier beforehand because themers could add things like borders and background and fixed height footers without worrying about those showing up if the footer wasn't being used.

[cash]

No footer, no applying an image to the footer. Let's pretend I'm using images to create some kind of fancy round corners with gradients and shadows and what not. I have a header element to apply the top to and a body element to apply the edges to but nothing for the footer.

As a themer, I shouldn't have to either override the module view or override every single call to the module view just to turn on the footer. The HTML should be there to support the themer.

[ewinslow]

You're talking about a use case that's basically rendered moot by CSS3. If we want to add elements that allow themers to add fancy stuff like this, it should not be the content blocks. E.g. see OOCSS and the <b> tags in modules.

I'm going to leave this closed because I don't want to turn it into a open/close war, but I really disagree with basically adding this extra markup in order to support something that most modern browsers can handle with CSS, and then breaking themes that expect the footer NOT to be there if there is no content.

[cash]

1. The module view already included the header regardless of content. Doing this with the footer makes it consistent. There was never an objection on the part of the header.

2. I consider the 1.8.1 release the finalized 1.8 release. No more changes to stuff from here on out. Themers can expect the html to be consistent in the 1.8.x releases forward. If this were 1.8.2, I would agree with you on this point. Also, as far as I know, no one was using footers in modules (the core never adds footers). Was this objection based on a known issue with a theme?

3. The unfortunate reality is that adding the html to support this through images is still a common technique and required to handle some browsers. There's a reason OOCSS uses the top/bottom <b> tags (which we never adopted).

4. There is no perfect markup. We are always going to bias the markup to work with one style of theming better than another. If a themer wants to put a border around the footer div and have it only show up when there is content, this makes it a little more difficult to accomplish that (they would have to use :empty).

[ewinslow]

1. Then we should not be including the header in these cases.
2. Then we should have RC's.
3. Then lets use that technique.
4. It might break my facebook theme.

[cash]

Okay we agree that we should be consistent with header and footer in the module view.

Regarding 2, Elgg 1.8.0 was released a little prematurely with some significant bugs (embed plugin completely broken). A RC would have caught this. Some sort of test plan to do before a major release also would have caught this. We need to pick one of these.

It sounds like you are willing to compromise on adding html markup to support "fancy" modules (though you wish we would go all html5 and css3 and be done with it :). The two options are sticking with the current solution of using the header and footer div (which means they always need to be included) or using the OOCSS technique of empty <b> tags.

The argument for the current divs is that it is the smallest change to the markup. The argument for the <b> tags is that it separates html elements purely added for styling versus those that are semantic. An argument against the <b> tags is that it feels so dirty (<b class="top"><b class="tl"></b><b class="tr"></b></b>). The <b> option is so ugly that I'm reconsidering this altogether...

If we did neither, it would mean themers would need to insert the necessary html with JavaScript or override the module view to force the header/footers.

[ewinslow]

Hey, Cash,

Sorry for being so up tight about this. Thanks for being willing to work with me.

The <b> option is definitely dirty, but I think it affords the best compromise for everyone. :empty is not supported by IE8 and down, for example.

[cash]

I think there will continue to be cases were certain themes need non-semantic elements to achieve the effects the themer is looking for. CSS3 isn't going to satisfy all the current use cases.

Going over the options again:

1. Do nothing and make the themer override the view or insert them through JavaScript

2. Overload the header and footer divs for both content and themability

3. Add the separate non-semantic html

<b class="top"><b class="tl"></b><b class="tr"></b></b> feels so dirty to me that I'm willing to punt on this until 1.9 and see what demand there is for the non-semantic implementation. That would mean changing both head and foot to only appear if there is content.

[ewinslow]

SGTM thank you.

[Cash Costello]

Fixes #4003 footer in modules is included by default

Changeset: 5879ebf600db1609b0cd5112206fd1e34b722144