AusDTO / gov-au-ui-kit

MOVED TO https://github.com/govau/uikit/
https://github.com/govau/uikit/
MIT License
19 stars 12 forks source link

Conditional IE styles #343

Closed petronbot closed 8 years ago

petronbot commented 8 years ago

Description

This PR includes some changes to the CSS to make it possible to target specific versions of IE with styles directly in the SCSS:

  .govau--text {
    display: block;
    background-image: url('../latest/img/icons/logo-gov-au-2x.png');

    @include ie-lte(8) {
      background-image: url('../latest/img/icons/logo-gov-au-1x.png');
    }
}

There are 3 new stylesheets generated in the build, one each for IE6, 7 & 8. They can be included using conditional comments:

<!--[if IE 8]><link href="/latest/ui-kit-ie8.css" rel="stylesheet" type="text/css" /><![endif]-->

I've also restructured the SCSS folder to make dependencies more clear between partials:

gov-au-ui-kit/
└── assets/
   └── sass/
      ├─── components/
      ├─── utils/
      ├─── vendor/
      └─── ui-kit.scss

Definition of Done

klepas commented 8 years ago

@petronbot Want me to review this? At a glance this looks so awesome. :)

petronbot commented 8 years ago

@klepas Yes please, I've just done a rebase to fix conflicts so this now incorporates the latest from develop.

klepas commented 8 years ago

LGTM. Looks reallllly good.

I think we should devote some time to revisit the documentation setup after this refactor. What do you think @petronbot and @joolswood?

joolswood commented 8 years ago

For sure @klepas It'll be a substantial job.

Specifically with the IE templates, as this affects so many components, wondering whether we would document in it's own top level section, or include a reference within a part of every page, for example the 'Accessibility & browser support' accordion/section.

klepas commented 8 years ago

A11y and browser support are related, but I want to keep them separate from hereon in.

How verbose were you thinking of going with the IE conditional documentation? I think most of this could be done as commented sections within the code directly, imho.

joolswood commented 8 years ago

I think I need to sit down and walk through it with you @klepas

joolswood commented 8 years ago

As discussed with @klepas have logged a content debt ticket (DESIGN-431) for the restructured SCSS folder. Working on MVP text for the IE mixin.

klepas commented 8 years ago

LGTM.

petronbot commented 8 years ago

@klepas I've tested the conditional styles as working in IE6-8, do you think that's sufficient to tick off cross browser/device testing? And do you think this needs any design review?

klepas commented 8 years ago

[…] do you think that's sufficient to tick off cross browser/device testing?

Yup, considering it’s for IE.

And do you think this needs any design review?

Do we have anything specific to ask Gary to test out of this?

petronbot commented 8 years ago

Merge conflicts resolved and Changelog updated, will merge when build is finished.