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

"Open in new window" should be a page with no PL JS #265

Closed
EvanLovely opened this issue Feb 26, 2016 · 14 comments
Closed

"Open in new window" should be a page with no PL JS #265

EvanLovely opened this issue Feb 26, 2016 · 14 comments

Comments

@EvanLovely
Copy link
Member

I am using Pattern Lab Node v1.1.2 on Mac, using the grunt configuration.

Expected Behavior

When I view a pattern and click on "Open in new window" I should get an HTML page with no Pattern Lab assets so I can help troubleshoot to see if things like JS errors are from my code or conflicts with my code and Pattern Lab's JS. Currently this is loaded on that page and comes from ./source/_patternlab-files/pattern-header-footer/footer.html:

    <script>
        // handle injection of items from PHP
        var patternPartial = "{{ patternGroup }}-{{ patternName }}";
        var lineage = [{{{ lineage }}}];
        var lineageR = [{{{ lineageR }}}];
        var patternState = "{{patternState}}";
        var baseurl = "{{{baseurl}}}";
        var cssEnabled = false; //TODO
    </script>

    <script type="text/html" id="sg-pattern-html">
{{ patternHTML }}
    </script>

    <script type="text/html" id="sg-pattern-css">
{{ patternCSS }}
    </script>
    <script src="../../styleguide/js/vendor/jwerty.js"></script>
    <script src="../../styleguide/js/postmessage.js"></script>
    <script src="../../data/annotations.js"></script>
    <script src="../../styleguide/js/annotations-pattern.js"></script>
    <script src="../../styleguide/js/code-pattern.js"></script>
    <!-- /DO NOT MODIFY -->
Actual Behavior

I go to the "Open in New Window" page and see <script> tags other than I added.

Steps to Reproduce
  1. Clone site, run npm i && grunt serve
  2. Select a single components to go to.
  3. Up in the menu bar under the gear icon, click "Open in new window"
  4. Inspect element and look right before the close of the </body> tag.
@EvanLovely EvanLovely changed the title "Open in new window" should be a page with no PL assets "Open in new window" should be a page with no PL JS Feb 26, 2016
@bmuenzenmeyer
Copy link
Member

Hi Evan.

For what it's worth - this is how PL PHP has always worked, at least in the v1 world which Node is still emulating for the most part. I completely understand the use case you are describing. I'm open to suggestions as to how to make this better without throwing the baby out with the bath water.

Perhaps a different control / button / link other than the right-click context menu default behavior would meet this need.

@bramsmulders
Copy link
Contributor

Actually this isn't the case in pl-php for at least a couple of years now. They have really a separation of PL-ui and patterns. When we can close this and this the problem could be solved.

@bmuenzenmeyer
Copy link
Member

Thanks for the clarification - those two tickets are on deck.

@bmuenzenmeyer
Copy link
Member

@bramsmulders what version of PHP are you referring to when stating that this behavior does not occur? I am certainly not running latest PHP anymore but as of Pattern Lab PHP 0.7.0 which I am modeling the filesystem restructure off of - it appears to still add PL footer code - indeed it needs to in order to run.

Thanks!

@bmuenzenmeyer
Copy link
Member

cc @dmolsen

@bramsmulders
Copy link
Contributor

@bmuenzenmeyer, i'm pretty sure it is Patternlab-php 0.7.0. They have a pattern header and footer, but they also include _patterns/00-atoms/00-meta/ which are the start and end of a pattern. The pattern is always rendered with those meta templates, except when in viewall or styleguide mode. But i think @dmolsen can elaborate that more.

@bmuenzenmeyer
Copy link
Member

Ok. I am making those changes, and they are actually mostly live in https://github.com/pattern-lab/patternlab-node/tree/133 but what is throwing me for a loop is the assertion that there is no PL logic when opened in a new window. From what I know of 0.7.0, that's impossible. Need to keep looking perhaps - Dave always surprised me.

@mauricios
Copy link

Is this still on the roadmap?
Some Pattern Lab code doesn't validate using the w3c validation tools. Is there a workaround to generate the HTML pages without the Pattern Lab code?

@bmuenzenmeyer
Copy link
Member

@mauricios can you include the entire output of the offending pattern?

@mauricios
Copy link

@bmuenzenmeyer using the home page of the Pattern Lab demo http://demo.patternlab.io/patterns/04-pages-00-homepage/04-pages-00-homepage.html the w3c validation tools shows errors in the meta tags generated by Pattern Lab:

Error: Bad value cache-control for attribute http-equiv on element meta.
From line 13, column 1; to line 13, column 55
terns -->↩<meta http-equiv="cache-control" content="max-age=0" />↩<meta 

See the report: https://validator.w3.org/nu/?doc=http%3A%2F%2Fdemo.patternlab.io%2Fpatterns%2F04-pages-00-homepage%2F04-pages-00-homepage.html

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Sep 26, 2017

The demo site, last I heard, was built using Pattern Lab PHP.

cc @bradfrost

@bradfrost
Copy link
Member

The demo site, last I heard, was built using Pattern Lab PHP.

@bmuenzenmeyer that's correct, although I'd assume that the behavior should be the same here between Node and PHP. As I recall the cache stuff was there to ensure the browser didn't aggressively cache changes during development.

@mauricios
Copy link

As @bradfrost says the same headers appear in the Pattern Lab Node. In my test I can see that the meta tags are unnecessary since the Node server of Pattern Lab is already sending the HTTP headers for preventing the cache on the browsers.

This is the request headers of Pattern Lab Node:

➜  ~ curl -I http://localhost:3000/patterns/04-pages-00-homepage/04-pages-00-homepage.rendered.html
HTTP/1.1 200 OK
Accept-Ranges: bytes
Cache-Control: public, max-age=0
Last-Modified: Tue, 26 Sep 2017 16:13:16 GMT
ETag: W/"a68b-15ebef6c4af"
Content-Type: text/html; charset=UTF-8
Content-Length: 42635
Date: Tue, 26 Sep 2017 16:13:21 GMT
Connection: keep-alive

@bmuenzenmeyer
Copy link
Member

@mauricios

the content you are specifiying comes from https://github.com/pattern-lab/styleguidekit-mustache-default/blob/master/views/partials/general-header.mustache which is a dependency of PL PHP and Node.

If you don't like it you have a few options:

Your request is different than the original poster, however, so I'd ask that you bring your conversation somewhere else if we need to continue.

Back to the OP....

The homepage example is telling however, along with Brad's assertion that PL PHP output currently has all the additional code included too. It's just the way PL currently works, to support the iframe viewer and all its data processing and interaction, we ship each pattern snippet with logic needed to run in isolation. I think Brad has even written glue code for styleguide guide that removes this code.

I could see use cases for pattern export being "clean" and if I recall correctly, this is a feature / goal of that process.

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

No branches or pull requests

5 participants