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

Reduce snippet dependency#361

Merged
cshold merged 11 commits intomasterfrom
reduce-snippets
Mar 17, 2015
Merged

Reduce snippet dependency#361
cshold merged 11 commits intomasterfrom
reduce-snippets

Conversation

@cshold
Copy link
Contributor

@cshold cshold commented Mar 17, 2015

Evolution of #358

  • Place all one-time-use snippets inline
  • Combined snippets when possible (social media meta tags, collection sorting)
  • Exceptions:
    • Social meta tags (Open Graph and Twitter cards) are combined to one snippet
    • Old IE js support (Respond.js, shivs, polyfills)
    • Onboarding markup
    • Handlebars.js templates

This PR also:

  • Cleans up search result template to reduce duplicate code
  • Updates liquid variables to have spaces between `{{ }}``

cc @carolineschnapp @mpiotrowicz @stevebosworth @fredryk

Copy link
Contributor

Choose a reason for hiding this comment

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

can you add HTML comments denoting the start/end to the old snippets? I think it would go a long way in helping ppl navigate this file.

Eg

// begin mobile nav

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
Copy link
Contributor

this is great! Were any of the snippets un-snippified twice? (eg snippet contents appear twice now)

@cshold
Copy link
Contributor Author

cshold commented Mar 17, 2015

Nope, no un-snippification, aka no reused code that doesn't live in a snippet. I'll give it another once over to make sure that's the case though.

cshold added a commit that referenced this pull request Mar 17, 2015
@cshold cshold merged commit 1d52e68 into master Mar 17, 2015
@cshold cshold deleted the reduce-snippets branch March 17, 2015 15:24
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.

2 participants