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

Support for configuring a default pattern in config.json #270

Merged
merged 3 commits into from
Mar 7, 2016

Conversation

geoffp
Copy link
Contributor

@geoffp geoffp commented Feb 26, 2016

Summary of changes:
This adds basic support for setting the default pattern to be displayed when PL is first launched. We have a need for this because we want to have a nice landing page that explains things and doesn't melt mobile devices when they try to display every pattern we have all at once.

@bmuenzenmeyer, please review and let me know if there's anything glaringly wrong here.

@bmuenzenmeyer
Copy link
Member

Oooo, interesting. I definitely like the idea and remember it coming up before. The one thing to take note of is that by altering files like core/styleguide/js/styleguide.js and source/_patternlab-files/index.mustache we are consciously deviating from potential front-end parity with PL-PHP.

I've done it before and will probably do it again.

I think that's okay for now, FWIW, with two or three options down the road:

  • Port the feature into PHP / frontend styleguide code to make it "core"
  • Refactor this out as a plugin of sorts
  • Fork "core" styleguide-assets-default with a node flavor

Either way, I don't see denying this PR. Will take a look later this weekend.

@bmuenzenmeyer
Copy link
Member

An addendum to the readme would be a good shortterm fix - as patternlab.io matures, this may belong there too.

@geoffp
Copy link
Contributor Author

geoffp commented Feb 26, 2016

Kind of makes the case for consolidating our UIs ASAP. How close are we right now to UI compatibility? I saw that 1.1.3 closes some PHP gaps.

@bmuenzenmeyer
Copy link
Member

I haven't dug in to check for certain, but as I see it (for some reason please convince me otherwise I am agreeable 😄) I want to tackle the two items on https://github.com/pattern-lab/patternlab-node/wiki/Roadmap#v1xx-early-2016 first, especially finishing the file restructure. The first one is a legit gap that I want to close for feature parity too.

@geoffp
Copy link
Contributor Author

geoffp commented Feb 26, 2016

No, no -- you're right. It's probably best for the file/repo structure to be amenable to a truly pluggable UI before we do that.

Also, Pattern Engines are looking pretty good these days. We've had that branch (with a couple others mixed in, now including this one) in production for a week now, and the only hiccups we've had are with BrowserSync. Still working on that, I found a bug I need to fix and upstream. We can talk more about that later, though.

@@ -473,7 +473,11 @@ var patternlab_engine = function (config) {
var viewAllPathsPartialHtml = pattern_assembler.renderPattern(viewAllPathsTemplate, {'viewallpaths': JSON.stringify(patternlab.viewAllPaths)});

//render the patternlab template, with all partials
var patternlabSiteHtml = pattern_assembler.renderPattern(patternlabSiteTemplate, {}, {
debugger;
Copy link
Member

Choose a reason for hiding this comment

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

This should be removed, yes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes. 👍 EDIT: fixed

@bmuenzenmeyer
Copy link
Member

@geoffp did you know this browsersync option existed?

Me neither!

startPath: '/?p=pages-homepage',

Works like a dream. Does this adequately satisfy the aim of this PR?

@geoffp
Copy link
Contributor Author

geoffp commented Feb 29, 2016

Good find! I wish it did -- we need this mainly in static deployments that don't include BrowserSync.

@bmuenzenmeyer
Copy link
Member

Ah I understand - thanks for the clarification.

@bmuenzenmeyer
Copy link
Member

@geoffp poke about #270 (comment)

@geoffp
Copy link
Contributor Author

geoffp commented Mar 2, 2016

Errant debugger statement has been banished.

@bmuenzenmeyer bmuenzenmeyer added this to the v1.2.0 milestone Mar 7, 2016
@bmuenzenmeyer bmuenzenmeyer merged commit 9105c4b into pattern-lab:dev Mar 7, 2016
@bmuenzenmeyer
Copy link
Member

@geoffp this is in dev now. We will have to see how it shakes out with eventual reconciliation with styleguide-assets-default.

@geoffp
Copy link
Contributor Author

geoffp commented Mar 7, 2016

Cool. I'll volunteer to port it to "upstream" (we can call it that, right?) if and when the time comes.

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