Skip to content
This repository has been archived by the owner on Aug 30, 2018. It is now read-only.

Normalize #153

Merged
merged 20 commits into from Jul 29, 2014
Merged

Normalize #153

merged 20 commits into from Jul 29, 2014

Conversation

cshold
Copy link
Contributor

@cshold cshold commented Jul 22, 2014

First stab at normalizing the CSS, rather than resetting.

  • Removing the reset caused almost no visible issues, except for default list padding/margin
  • Moved some normalize code in to the sections where the rest of the attribute is defined
  • Also cleaned up shop.js' initialize call and cached selector setup

Resolves #151

carolineschnapp and others added 4 commits July 18, 2014 13:00
…iant...

Also, customers who have only 1 variant may still want to show the
variant title if it's not set to 'Default Title' so I am removing the
condition altogether rather than fixing it.
@@ -733,10 +755,7 @@ h1, h2, h3, h4, h5, h6,
h1 a, h2 a, h3 a, h4 a, h5 a, h6 a,
.h1 a, .h2 a, .h3 a, .h4 a, .h5 a, .h6 a { font-weight: inherit; }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mpiotrowicz Should I just extend h1 from .h1 (and so on), rather than duplicate up the selectors here and below?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I'd probably go as follows:

h1,h2,h3,h4,h5,h6 {
  a {
    font-weight: inherit;
  }
}

and then have all the typography helpers extend the tags, so

.h1 {
  @extend h1;
}

That would include the link inside .h1 as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Am I the devil if I keep those extends on one line?

.h1 { @extend h1; }
.h2 { @extend h2; }
.h3 { @extend h3; }
.h4 { @extend h4; }
.h5 { @extend h5; }
.h6 { @extend h6; }

It goes against the styleguide, but it's so pretty.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd stick with the guide for consistency. This style works but as soon as anything has to change, it would have to move to 2 lines anyway.

cshold added a commit that referenced this pull request Jul 29, 2014
Normalize CSS and Clean shop.js
@cshold cshold merged commit a4b2eb5 into master Jul 29, 2014
@cshold cshold deleted the normalize branch July 29, 2014 17:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove CSS reset
3 participants