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

Adding routine to copy js along pattern build #798

Merged
merged 3 commits into from
Feb 16, 2018

Conversation

raphaelokon
Copy link
Contributor

Addresses #717

Summary of changes:

  • Adding copy routine to copy pattern-specific JavaScript in place

@coveralls
Copy link

coveralls commented Feb 15, 2018

Coverage Status

Coverage increased (+0.04%) to 73.224% when pulling 351ea5e on feature/copy-pattern-js into 98b24fa on dev.

@bmuenzenmeyer
Copy link
Member

@raphaelokon this works pretty slick - nice work!

This is the original output:

image

I've changed it slightly so the file name output is the same as one would include the partial itself:

image

This has the following benefits.

  1. Muscle-memory for inclusion
<script src="atoms-brands-colors.js"></script>
  1. Users can shortcut this by doing the following instead!
<script src="{{patternPartial}}.js"></script>

This works because I am renaming the file during copy, as you'll see in my commit, and then explicitly adding patternPartial to the data passed to render

The source files still follow the same convention (same name, different ext):

image

BONUS ✨

By adding patternPartial to the data - we can support conditional CSS or (perhaps verbose JS) in the meta patterns

<body class="{{ bodyClass }} {{ patternPartial }}">
<script>if ('{{patternPartial}}' === 'pages-splash') { ... }</script>

though I dont know if the later is a good idea ⚔️

@bmuenzenmeyer bmuenzenmeyer merged commit bcf60e4 into dev Feb 16, 2018
@bmuenzenmeyer bmuenzenmeyer deleted the feature/copy-pattern-js branch February 28, 2018 12:32
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

Successfully merging this pull request may close these issues.

3 participants