Attached is a patch that changes the use of "

" to class="clearfix" in page.tpl.php where applicable.

The use of clearfix is now a Drupal standard and is a part of Drupal core: http://drupal.org/node/254940#clearfix

You are already using clearfix in node.tpl.php so I assume you are comfortable / familiar with this approach.

There were two instances of "

" that I left in the page.tpl.php in this patch because the clearfix solution wouldn't necessarily apply.

And thanks for this great-looking theme! It came in really handy for us in a pinch.

CommentFileSizeAuthor
#4 danland-1047388-3-whitespace.patch17.5 KBtim.plunkett
#4 danland-1047388-3.patch5.35 KBtim.plunkett
danland-clearfix.patch17.22 KBacouch

Comments

danpros’s picture

danpros commented
Version: 7.x-1.0-rc2 » 7.x-1.0
Category: bug » task

Hi,

Thanks for the patch. But its different.. I use global fix for this, the D6 is using <div class="clear-block">, D7 using <div class="clearfix"> but I am using <div style="clear:both"></div>. That will always work in any Drupal version since this is CSS standard.

Dan,

acouch’s picture

acouch commented

hi Dan,

Using <div class="clearfix"> is considered a best practice because it doesn't use the extra markup that using <div style="clear:both"></div> does. '

' is not really a CSS solution like clearfix is because it requires extra markup.

All of the places I removed the <div style="clear:both"></div> are instances where the 'clearfix' is added to an already existing div.

Your theme is visually beautiful and I would like to help make the template match!

danpros’s picture

danpros commented
Status: Active » Patch (to be ported)

Hi Aaron,

Sorry for fast replay but without thinking it first, I have a lot of issue queue and I hate ignoring people, so sometimes my answer will look like silly :)

You are right it will add an extra markup and that is not a "best practice", okay I will add this fixes in my next release. Hey maybe you can adding new features to Danland as well? You have done a great work for the community :)

Dan

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Patch (to be ported) » Needs review
StatusFileSize
new danland-1047388-3.patch5.35 KB
new danland-1047388-3-whitespace.patch17.5 KB

Hardcoding empty divs is never good, for both theming and semantic reasons. And doing it for the reason of compatibility between D6 and D7 isn't a good reason.

There were some weird whitespace issues in the last patch. I've attached two, one with just the required changes, the other with whitesepace fixes.

danpros’s picture

danpros commented
Status: Needs review » Patch (to be ported)

Hi Tim,

Wow we submitted a post on the same time, but I won a few second there. Congrats Dan!

Yes that is not a good reason :). I check the patches and the codes become very clean (not nasty). I will use all of this fixes for the next release. I will mark this as patch to be ported (for reminder). I will implemented it later, if somethings goes wrong I will post it in here.

Thanks,
Dan

acouch’s picture

acouch commented

Thanks Dan.

Again, great theme.

One thing to note that with the 6.x Drupal used clear-block instead of clearfix: http://drupal.org/node/254940#clearfix

You could still use clearfix and just add that to your theme css.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Patch (to be ported) » Needs review

I don't think either of these were committed?

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Needs review » Closed (won't fix)