Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document incuna sass #49

Open
5 of 27 tasks
hippogriffic opened this issue Aug 12, 2015 · 11 comments
Open
5 of 27 tasks

Document incuna sass #49

hippogriffic opened this issue Aug 12, 2015 · 11 comments

Comments

@hippogriffic
Copy link
Contributor

Here is the list of things that need documentation. For documenting style reference see https://github.com/incuna/incuna-sass/blob/master/incuna-sass/mixins/_svg.sass

Documentation should explain what the code does, why it's needed and provide examples of when / how it can be used.

If the file is not something we use anymore / need for currently supported browsers please make a note of this as well.

Mixins

Helpers

Functions

Components

Root

@incuna/frontend please find some time to look these over and add some documentation. Alternatively you can come and tell me what it does / why we need it and I'll add the docs.

@pandalion
Copy link
Contributor

I'll add some brief notes here and then whoever gets chance first can update the files themselves with descriptions where necessary.

Mixins

  • Font-face - still needed. We should revisit any changes in browser requirements for web fonts though and if this needs to be changed at all. And we should improve the docs a bit.
  • media - just a short way to do media queries. We should comment it a bit more. I'm less familiar with the hidpi thing, so maybe we need to comment this more too.
  • placeholder - for form placeholders, needs checking if needs amending. Update - just discussed and the styling of placeholders is not recommended by w3c anymore so we should check this and we might be able to remove this file.
  • rgba background - looks like something only needed for ie8 maybe. so we could move this kind of thing to a different folder like a "ltie9" folder or something, perhaps. Or a "legacy" folder?

Helpers

  • box-sizing - don't remember much about this, maybe it's shorthand for the box-sizing properties? Would need to check.
  • clearfix - this is to clear floats without putting in markup just to clear them. Because if you have a floated element, and it doesn't have something after it, it won't take up any space. One way is to add something after it, with a "clear: left" property, this mixin allows you to avoid adding extra elements just to clear the float. I would like to see if there is a more updated way to do this.
  • image-replace - so we can have text in the html for something like a logo, but then hide the text. I wonder if this is the best way to do it. Would like to know what @ninjanails thinks too. For example, if we have a logo <div class="site-logo">Site name</div> and hide the text, is that the best thing. Or should we be displaying the logo and making the site name known in another way?
  • inline-block - only need worrying about for ie6 and 7. For the very minor amount of sites we do with this in, we could remove it. If we want to keep it for older sites, we could separate it, eg in the "legacy" folder
  • link - fairly self explanatory tho we could comment it
  • overflow-scroll - only needed for touch things i think. it would be good to maybe also have a folder of incuna-sass stuff that's needed for touch devices.
  • pseudo-elements - fake before/afters for IE7 because it didn't support them. something for legacy folder again?
  • vertical-center - ha I never even knew we had this (or I forgot). I wonder if it works. vertical centering is always a pain. Not too familiar with this method.
  • visually-hidden - comments would have been useful on this one! As I'm not sure what it's for at all XD

Functions

  • em and rem - we could comment this better, I've always found it super hard to explain what the rem one is doing even tho I understand !

Components

  • angular - yeah I think this comment is enough? but can change if it's not clear
  • datepicker and flowplayer - these go with the standard plugins we use for datepickers and videos. So we could comment this to be clearer.
  • forms - I think this is commented quite well already, but it's worth us going through and seeing if anything is irrelevant now
  • icomoon - stuff for icon fonts hides from seren - it might be worth keeping for now for old projects, but we could comment it a bit more, or move it to legacy.
  • messages - was used as a default style for django messages which were all the kind of confirmation messages that would appear on a page, eg "Profile updated", "User deleted." I would be interested to check the structure that messages have in our angular projects and if this matches this still, it's definitely a useful default to have but I'm not sure if all the class names in here are still correct.
  • modal - some kind of popup window/overlay. i think we have been avoiding these in our angular sites for the most part, but this might still be useful for default styles if we do need one. Would be good to check over it again to see if it needs updates anyway.

Root

  • browsers - this was using the compass browser detection to decide which properties to use. I think we'll have to get rid of this and replace how we're doing it. I think possibly the idea below of a legacy folder, or an "edetail" folder, and just putting all helpers/mixins needed for those in those folders could replace doing it this way.
  • normalize and reset - I'm not too sure why we have two of these. These files are usually used to remove browser defaults and give the styling a standard base to start from. But I'm not sure if these could be combined into one file?

Generally, what do @incuna/frontend think about the idea of making IE9 the minimum as a standard for stuff , as it is for most projects, and then separating other stuff to a "legacy" folder for older browsers. So we aren't actually getting rid of it, but on a project, you could include @import incuna-sass/legacy if you need the older things. Because I think a lot of this won't be needed for our ie9 minimum projects anymore.

@pandalion
Copy link
Contributor

Thinking about webfonts i think these days we might only need woff and ttf.

@maciejpi
Copy link
Contributor

beth, we should check what bourbon has. I was going to do this this morning but i'm not on myday anymore.
but roughly:

placeholder: may be needed for IE9?
box-sizing: not needed
clearfix - in bourbon.
inline-block - not needed
em &rem - in bourbon

@maciejpi
Copy link
Contributor

@incuna/frontend

Please see the review of some of the incuna-sass elements and how they relate to bourbon.

my recommendation are in bold (of course the final decision belongs to the team:) )

font face

you can specify weight, style and file formats in bourbon mixin

@include font-face("source-sans-pro", "/fonts/source-sans-pro/source-sans-pro-italic", normal, italic)
@include font-face("source-sans-pro", "/fonts/source-sans-pro/source-sans-pro-regular", $file-formats: eot woff2 woff)

consider switching to bourbon mixin and remove incuna-sass


media

there's no media mixin in bourbon (it's in neat - bourbon-related project) so we need ours. but there's HIDPI media mixin targeting retina displays. Our media mixin targets retina as well but perhaps we could update the $hidpi variable with pixel ratio that's in bourbon hidpi mixin(?)

use incuna-sass mixin but consider updating hidpi bit


placeholder

the same name, the same functionality, and almost the same code

consider using bourbon mixin and remove incuna-sass


rgba-background

there's nothing like that in bourbon but sass has a built-in function for that that works like that:
background: rgba(#111, 0.4)
Our mixin has a fallback for IE8

move the IE8 fallback to incuna-sass-legacy and remove the mixin from incuna-sass


box-sizing

The box-sizing mixin has been deprecated and will be removed in in the next version of Bourbon!
Box sizing is widely supported.
Remove the helper from incuna-sass or move to legacy-project


clearfix

bourbon has its own clearfix that doesn't use :before
our helper supports IE7. If there's IE8+ support It looks like using before is not necessary: http://cssmojo.com/latest_new_clearfix_so_far/

use shorter and more modern bourbon mixin and, if needed, create IE7 clearfix mixin in incuna-sass-legacy


image-replace

Bourbon uses hide-text mixin that has similar functionality but for block-level elements Bourbon requires the element to have display: block/inline-block set

p
    @include hide-text
    display: block / inline-block  //  <-- this is just for block elements

Our helper has IE7 fallback and block functionality included
Consider removing/modifying incuna-sass helper and moving IE7 functionality to legacy project


inline-block

This stuff is for IE6-7
move it to legacy project


link

nothing like that in bourbon. let's keep it?


overflow-scroll

no duplicate in bourbon. This helper uses webkit-specific feature.
make sure to use it only in webkit-specific projects (edetail)


pseudo-elements

No bourbon alternative
targeting IE7. Move to legacy project


vertical-center

nothing like that in Bourbon.
I tried to play with our helper but I really don't get what it does.

Can we remove it?


visually-hidden

no duplicate in Bourbon.
I'm not sure what's the aim of this helper. It certainly hides an element but to do that we can just use position: absolute, top: -9999px or something like that.

Shall we remove it?


em & rem

Bourbon has the functions with the same names and functionality. The only difference is that Bourbon allows the argument to be unitless. However, it also accepts units (to make happy all our unit-supporters ;) )

Remove our rem and em

@hippogriffic
Copy link
Contributor Author

@incuna/frontend can I get some other people's opinions on the following please:

if you think we need these please explain why (and what they do / when they can be used) otherwise I'm going to suggest we remove them because I don't know what they're for :)

@perry
Copy link
Collaborator

perry commented Aug 19, 2015

Visually hidden is largely taken from http://snook.ca/archives/html_and_css/hiding-content-for-accessibility

@perry
Copy link
Collaborator

perry commented Aug 19, 2015

Vertical center will vertically center a child element within the parent, eg

.parent 
    @extend %vertical-center
.child 
    @extend %inline-block

@perry
Copy link
Collaborator

perry commented Aug 19, 2015

Overflow scroll enables momentum scrolling on overflowed elements in iOS

@hippogriffic
Copy link
Contributor Author

@perry can you help me understand the difference between visually-hidden and image-replace as in when would we use them and why we need both?
I can't really get my head around the difference

@perry
Copy link
Collaborator

perry commented Aug 19, 2015

Sure. image-replace will visually hide text within an element, whilst keeping the block structure. visually-hidden will visually hide the entire element, whilst still keeping the text available to screen readers.

@hippogriffic
Copy link
Contributor Author

thank you, that's really helpful :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants