I noticed @JohnAlbin added an arguments to _base.scss, which enable underscore & star hacks in various locations based on whether or not the user wishes to support legacy IE versions.

Is this in an effort to rid Zen of conditional stylesheets?

We could move the standard IE conditional CSS files over to underscore/star hacks if that's the direction we're going. But perhaps it's just in an effort to provide the best Sass/Compass experience possible.

Perhaps ie classes should be moved to the html element:

http://paulirish.com/2008/conditional-stylesheets-vs-css-hacks-answer-ne...

CommentFileSizeAuthor
#11 remove-ie-conditional-stylesheets-1239390-8.patch19.01 KBKrisBulman
#8 conditional-html-class-1239390-8.patch2.2 KBJohnAlbin
#2 conditional-html-class-1239390-2.patch1006 bytesJohnAlbin
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Title: ie conditional stylesheets vs star & underscore hacks » conditional stylesheets vs css hacks vs conditional classes

I dislike CSS hacks. However, I added those into the Sass stylesheets because there was no way to add IE support into a single mixin. To use conditional stylesheets with Sass would require having parallel mixins for IE stylesheets. That just underscore how awkward separate conditional stylesheets are.

Here are our options:

  1. CSS hacks: omgawful. brittle and are as easy to understand as a single-line Perl script.
  2. IE conditionally included stylesheets: easily understood, no CSS hacks like #1, but physically separated from the non-IE styles, which is awkward.
  3. IE conditionally classed html tag: advantages of #2 and IE styles can be placed right next to their non-IE counterparts. But I’ve been hesitant to switch to the method because of D7's RDFa namespaces which are rendered as part of the HTML tag.

Imagine this:

<!--[if lte IE 6]><html class="ie6" lang="en" dir="ltr"
  xmlns:content="http://purl.org/rss/1.0/modules/content/"
  xmlns:dc="http://purl.org/dc/terms/"
  xmlns:foaf="http://xmlns.com/foaf/0.1/"
  xmlns:og="http://ogp.me/ns#"
  xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#"
  xmlns:sioc="http://rdfs.org/sioc/ns#"
  xmlns:sioct="http://rdfs.org/sioc/types#"
  xmlns:skos="http://www.w3.org/2004/02/skos/core#"
  xmlns:xsd="http://www.w3.org/2001/XMLSchema#">><![endif]-->
<!--[if IE 7]><html class="ie7" lang="en" dir="ltr"
  xmlns:content="http://purl.org/rss/1.0/modules/content/"
  xmlns:dc="http://purl.org/dc/terms/"
  xmlns:foaf="http://xmlns.com/foaf/0.1/"
  xmlns:og="http://ogp.me/ns#"
  xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#"
  xmlns:sioc="http://rdfs.org/sioc/ns#"
  xmlns:sioct="http://rdfs.org/sioc/types#"
  xmlns:skos="http://www.w3.org/2004/02/skos/core#"
  xmlns:xsd="http://www.w3.org/2001/XMLSchema#"><![endif]-->
<!--[if IE 8 ]><html class="ie8" lang="en" dir="ltr"
  xmlns:content="http://purl.org/rss/1.0/modules/content/"
  xmlns:dc="http://purl.org/dc/terms/"
  xmlns:foaf="http://xmlns.com/foaf/0.1/"
  xmlns:og="http://ogp.me/ns#"
  xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#"
  xmlns:sioc="http://rdfs.org/sioc/ns#"
  xmlns:sioct="http://rdfs.org/sioc/types#"
  xmlns:skos="http://www.w3.org/2004/02/skos/core#"
  xmlns:xsd="http://www.w3.org/2001/XMLSchema#"><![endif]-->
<!--[if gte IE 9]><!--><html lang="en" dir="ltr"
  xmlns:content="http://purl.org/rss/1.0/modules/content/"
  xmlns:dc="http://purl.org/dc/terms/"
  xmlns:foaf="http://xmlns.com/foaf/0.1/"
  xmlns:og="http://ogp.me/ns#"
  xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#"
  xmlns:sioc="http://rdfs.org/sioc/ns#"
  xmlns:sioct="http://rdfs.org/sioc/types#"
  xmlns:skos="http://www.w3.org/2004/02/skos/core#"
  xmlns:xsd="http://www.w3.org/2001/XMLSchema#"><!--<![endif]-->

:-(

But I just realized that the RDF namespaces only need to be on the non-IE conditional HTML tag and not on any of the others. An RDF consumer would not be using IE's rendering engine to parse the HTML.

Shall we make Zen use Paul Irish's method then? :-)

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Active » Needs review
FileSize
conditional-html-class-1239390-2.patch1006 bytes

A patch, perhaps?

hansyg’s picture

hansyg CreditAttribution: hansyg commented

+1 I think this is the best solution

dcmouyard’s picture

dcmouyard CreditAttribution: dcmouyard commented

+1

Definitely go with #3, à la HTML5 Boilerplate.

KrisBulman’s picture

KrisBulman CreditAttribution: KrisBulman commented
Status: Needs review » Reviewed & tested by the community

patch applied

$ git apply -v conditional-html-class-1239390-2.patch
Checking patch templates/html.tpl.php...
Applied patch templates/html.tpl.php cleanly.

view source

<!DOCTYPE html>
<!--[if IEMobile 7 ]><html class="iem7" lang="en" dir="ltr"><![endif]-->
<!--[if (lt IE 7)&!(IEMobile) ]><html class="ie6" lang="en" dir="ltr"><![endif]-->
<!--[if (IE 7)&!(IEMobile) ]><html class="ie7" lang="en" dir="ltr"><![endif]-->
<!--[if IE 8 ]><html class="ie8" lang="en" dir="ltr"><![endif]-->
<!--[if (gte IE 9)|(gt IEMobile 7) ]>--><html lang="en" dir="ltr"
  xmlns:content="http://purl.org/rss/1.0/modules/content/"
  xmlns:dc="http://purl.org/dc/terms/"
  xmlns:foaf="http://xmlns.com/foaf/0.1/"
  xmlns:og="http://ogp.me/ns#"
  xmlns:rdfs="http://www.w3.org/2000/01/rdf-schema#"
  xmlns:sioc="http://rdfs.org/sioc/ns#"
  xmlns:sioct="http://rdfs.org/sioc/types#"
  xmlns:skos="http://www.w3.org/2004/02/skos/core#"
  xmlns:xsd="http://www.w3.org/2001/XMLSchema#"><!--<![endif]-->

<head profile="http://www.w3.org/1999/xhtml/vocab">
JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Status: Reviewed & tested by the community » Needs work

I missed something in the patch. The "-->" is rendered in IE9.

Glad to see favorable responses so far. Anyone else? :-)

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

Just discovered the reason for the bug. See http://en.wikipedia.org/wiki/Conditional_comment#Downlevel-revealed_cond...

Re-rolling the patch now and cleaning up some of the conditions.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Issue summary: View changes

updated name

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Title: conditional stylesheets vs css hacks vs conditional classes » Add conditionally-classed HTML tag and remove IE conditional stylesheets
FileSize
conditional-html-class-1239390-8.patch2.2 KB

I just committed this patch. Which fixes the earlier bug and adds a couple additional IE-specific classes.

But we need follow-up work to convert the Sass IE CSS hacks to use these new html classes. And to remove the bits of Zen that supported conditional stylesheets. (If you still want to use them, you can download the Conditional Stylesheets module.)

KrisBulman’s picture

KrisBulman CreditAttribution: KrisBulman commented

Great! should we open a new ticket to submit patches as the stylesheets are fixed up, or add to this one.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

@Kris, you can just submit to this one. :-)

Also, the conditions that I used are based on my research into 320 and up. However, I think there's some logic errors in that code, so I fixed it, committed it to Zen and posted a patch to 320 and up. Discussion of my proposed improvements are at: https://github.com/malarkey/320andup/pull/4

KrisBulman’s picture

KrisBulman CreditAttribution: KrisBulman commented
Status: Needs work » Needs review
FileSize
remove-ie-conditional-stylesheets-1239390-8.patch19.01 KB

ok, this should take care of the ie6 & ie7 stylesheets, while retaining support for adding/removing ie support via keyword arguments in _base.scss. It also updates the css stylessheets and disables them from the .info file

I wasn't sure how to handle the clearfix, element-invisible or unpublished div in _base, please provide direction.

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Category: support » feature
Status: Needs review » Fixed

Thanks, Kris!

BTW, the zen-columns-fixed mixin already had IE6/7 support in it, so we didn't need to create a new mixin for it. I did convert its ie6/7 CSS hacks to proper .ie6-7/.ie6 classes instead.

I've taken what you've done and reviewed, modified and committed it in chunks to make it easier to see what is going on.

http://drupalcode.org/project/zen.git/commit/a28855c
http://drupalcode.org/project/zen.git/commit/9534687
http://drupalcode.org/project/zen.git/commit/82f2d85
http://drupalcode.org/project/zen.git/commit/12af091
http://drupalcode.org/project/zen.git/commit/83d51b9

KrisBulman’s picture

KrisBulman CreditAttribution: KrisBulman commented

nice.. first, and hopefully not last, mention in a drupal commit!

thanks for the info, pulling apart the mixin taught me a lot about how it worked anyway, so it was worth the effort.

Status: Fixed » Closed (fixed)

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

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented

Updated the conditions based on further conversation in https://github.com/malarkey/320andup/pull/4

JohnAlbin’s picture

JohnAlbin
he/they/他
Primary language English
Location Taipei, Taiwan
CreditAttribution: JohnAlbin commented
Issue summary: View changes

Corrected my wording when referring to Paul Irish's method