search

pattern-lab / the-spec

The specification for implementing Pattern Lab in various languages. This way there can be common core functionality and common shared assets.
16 stars 3 forks source link

Discussion and Vote: Pattern State Declaration, and other Pattern Metadata #16

Closed bmuenzenmeyer closed 8 years ago

bmuenzenmeyer commented 8 years ago

As a Pattern Lab Node user, I use a configuration json file to define pattern states. Am I compliant with the spec?

Pattern States in PL/Node use a key/value object defined in patternlab-config.json instead of the PHP filename approach, which uses filename suffixes.

Here's an example from the docs:

"patternStates": {
  "molecules-single-comment" : "complete",
  "organisms-sticky-comment" : "inreview",
  "templates-article" : "complete"
}

This was a conscious decision made at time of implementation, with the following benefits:

This deviation would, however, make someone porting from PL/* to the other a bit more cumbersome. Though, pattern states might be low on their priority list when switching anyways.

What I am asking with this question is whether or not this deviation from PHP is in spec.

The timeline for this feature is dependent on the outcome of this issue, as spec compliance is a goal of PL/Node.

Tagging spec-question because I need input regarding compliance, and am not proposing a change to PHP's strategy.

This issue does not require a vote in my opinion, but we should perhaps open a process question to determine how questions like this are handled officially in the future. I'll wait to create one until after this issue shakes out.

JUNE 3RD EDIT

/cc @pattern-lab/voting-members

bradfrost commented 8 years ago

I like this. I'm thinking about version control with how pattern states are currently set up. If changing a state requires changing the file name, then it becomes harder to track and step through the history in a version control system. Am I right about that? I haven't actually used pattern states in projects. Just a thought.

bmuenzenmeyer commented 8 years ago

As far as I know, some version control systems can recognize the rename while others cannot, which would result in delete/add's and loss of history.

I am curious which method is preferred at scale, or if it matters, and both implementations could be considered in spec, the important part being the actual functionality, not the details.

I envision that's exactly why we are having these discussions :dart:

dmolsen commented 8 years ago

It's definitely not in spec. Sorry. In spec would mean someone could copy a set of files from one to the other and given the same input they'd get the same output.

That said, I'd be willing to deprecate the @status feature in favor of something like Fractal's statuses methodology which is part of their component configuration. While your points are valid I think an entirely separate file goes too far the other way as it's really indirect.

If we go the route of copying parts of "component configuration" (not all are applicable) someone should be able to set it in a pattern's .json, .yml, or .md file. The latter as part of the YAML front-matter.

Would this provide a happy medium between a fully separate config file and @status?

bradfrost commented 8 years ago

@dmolsen I totally dig having all that pattern-specific stuff as part of the Front Matter!

-----
title: Media Object
description: This is the description.
status: inprogress
------

Love it.

dmolsen commented 8 years ago

Well, description is just the markdown portion of the file and wouldn't come from the front-matter. But something like order could.

On Jun 1, 2016, at 3:23 PM, Brad Frost notifications@github.com wrote:

@dmolsen I totally dig having all that pattern-specific stuff as part of the Front Matter!

title: Media Object description: This is the description.

status: inprogress

Love it.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

bmuenzenmeyer commented 8 years ago

Would this provide a happy medium between a fully separate config file and @status?

I think putting status into the Front Matter makes a lot of sense. It would be easier than editing a filename and closer linked to the pattern it was declaring status for than tucked away inside a massive config file.

It also might make things like referenced starterkits easier to consume.

A separate discussion should be opened about whether to move order into this file too. I know that using order baked into the file name has just as many problems as status, (if not more!) and it only gets worse with scale.

So...

?

dmolsen commented 8 years ago

Too keep this somewhat clean let's just roll this into one discussion & vote issue. Here are the four I like from Fractal:

In addition to that I could see an option that allows a pattern to be in the nav but not shown on a "view all" page.

Why don't you review the Fractal docs and see if anything else pops out for you. "Collections" is our subtype so maybe there are options there too.

bmuenzenmeyer commented 8 years ago

tags

Oooooo.... interesting. Imagine a search that is not only pattern name aware, but tag aware too. I like this a lot.

hidden

This would deprecate the _pattern.mustache approach then?

In addition to that I could see an option that allows a pattern to be in the nav but not shown on a "view all" page.

Yes that would be a nice place to do this! We do this in PL/Node today with a styleguideExcludes array that lists the patternPartials to skip when building the view-all pages. I think PL/PHP has the same?

Why don't you review the Fractal docs and see if anything else pops out for you. "Collections" is our subtype so maybe there are options there too.

Will do and report back

bradfrost commented 8 years ago

I like this. I'm also wondering if there's a way to do a freeform key-value pair thing that lets people extend this info? Thinking things like:

If the key matches something PL uses, it would behave accordingly, but if not it would dump it in the panel.

This is just a thought, but figured now would be the time to bring it up since we're talking about pattern info like this.

dmolsen commented 8 years ago

That's supposed to work in PL/PHP now but somehow I've borked that. That's what's supposed to go in {{# descAdditions }} or whatever that block is called in the base template. Good that you brought it up because it should get documented for spec. And fixed ;)

On Jun 2, 2016, at 7:32 PM, Brad Frost notifications@github.com wrote:

I like this. I'm also wondering if there's a way to do a freeform key-value pair thing that lets people extend this info? Thinking things like:

Version (of a specific pattern i.e. 🔥 CAROUSEL 3.0 🔥 People (i.e. Sarah Smith owns this pattern) Revision Date Other stuff If the key matches something PL uses, it would behave accordingly, but if not it would dump it in the panel.

This is just a thought, but figured now would be the time to bring it up since we're talking about pattern info like this.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

bmuenzenmeyer commented 8 years ago

@bradfrost - response

I'm also wondering if there's a way to do a freeform key-value pair thing that lets people extend this info?

neat idea. right now I am white-listing the parsing of frontmatter in PL/Node, but yes one could let anything that isn't an exact key match fall through to what Dave is alluding to. If that's intended behavior I will need to open an issue to do that.

@dmolsen - response

Why don't you review the Fractal docs and see if anything else pops out for you. "Collections" is our subtype so maybe there are options there too.

The ones you chose look good to me too. display is interesting to purposefully constrain an element that you know is typically to be seen in a small viewport, and say, is styled using flexbox, and looks odd when the iframe extends beyond the pattern's typical use. I think we have mechanisms to achieve the same effect however, like the ish controls or styleguide-specific.css Don't know if it merits inclusion though.

Please also respond to https://github.com/pattern-lab/the-spec/issues/16#issuecomment-223285625 questions regarding hidden and the styleguideExclude option.

I'll attempt to summarize what we are voting on by editing the top-most comment.

dmolsen commented 8 years ago

This would deprecate the _pattern.mustache approach then?

I'd like to keep this just because I think it makes it easy to tweak something fast. Hidden is going to happen more often than a status change. And Fractal supports both methods so I have a feeling there's value there. From a spec perspective, if "hidden" is flagged in either the .md or via an _ then the pattern is hidden.

Yes that would be a nice place to do this! We do this in PL/Node today with a styleguideExcludes array that lists the patternPartials to skip when building the view-all pages. I think PL/PHP has the same?

Yes. It also supports a dash - to hide things from a style guide but include in the nav. The attribute that the dash sets is noviewall. Not saying that has to be the prop name but throwing it out there. I think excludeFromStyleguide is probably the best even if crazy long.

re:random key/value pairs. If that's intended behavior I will need to open an issue to do that.

It's the intended behavior. So from a "system input" side of things it's just tacking on the array. I'll have to document the "system output" side of that for this issue.

display... Don't know if it merits inclusion though.

I don't think our set-up will support it. I honestly haven't looked at Fractal's output but I'm going to go out on a limb and say each pattern is put into its own iFrame. Therefore they can set dimensions on it to control that kind of thing. We definitely don't have that ability.

I'll attempt to summarize what we are voting on by editing the top-most comment.

Thanks for that.

parse excludeFromStyleguide (or a better name) out of a pattern's .md Front Matter and use to omit pattern from all rendering

That should be "any rendering for a View All view."

parse tags out of a pattern's .md Front Matter and ... ??? (available in a future search enhancement?)

Let's just leave this is an input for now. We can determine output at a later date. Maybe it's "View All" by tag or just hooks for plugins.

Because I mentioned it above I'm now supporting only doing this in the .md file. .json/.yml support would be nice but I think it'd a) get tricky and b) not offer much over the filesystem changes or the .md.

dmolsen commented 8 years ago

One last idea. If it gets an ok I'll resummarize @bmuenzenmeyer's summary as an input/output block and then vote. Drizzle adds the notion of "links" that can be associated with a pattern. For example, a link to a description of an API call or similar. It's a simple object with a label and url property. It looks like:

links:
      - label: Demo
        url: "#"
      - label: Article
        url: "#"
      - label: Source
        url: "#"

That gets outputted along with the rest of the description stuff. I think it's a nice way of offering a clean method to further document the what and why of a pattern. Sound ok?

bmuenzenmeyer commented 8 years ago

Sounds straightforward.

@dmolsen Would these be used as opposed to someone having to format their description to always include this same markdown snippet?

These links would need actual real-estate on the pattern info left-hand side, yes, complete with markup and styling? Or could they be placed next to the pattern name and state on the top, to display a JIRA/Ticket number and the like?

dmolsen commented 8 years ago

@bmuenzenmeyer -

Thought I had replied. Yes, they would need space on the left-hand panel with Brad's styling.

Tacking on a JIRA number in that very specific space would be a plugin to me. I hate to say "plugin" to most everything but at least that one could be created relatively easily. The question is how to expose that data so it's not repetitive. I'm working on cleaning up my data output now and will propose a solution for that.

Maybe links is one attribute to far. I'm ok with that.

bmuenzenmeyer commented 8 years ago

@dmolsen I think my point was lost - I apologize. I took a peak at Drizzle and I think the links you speak of are the MDN documentation here, yes? If so, I am all for this, as it would give Pattern Lab the ability to assign arbitrary links to patterns in a system-driven way, rather than having to remember to put the same sorta formatted links in every patternDescription markdown block that applied (like a JIRA ticket, only an example!).

So, that's my long winded way of saying this is a great idea. I'm ready for a summary and vote.

dmolsen commented 8 years ago

👍

bmuenzenmeyer commented 8 years ago

:+1:

bmuenzenmeyer commented 8 years ago

After a discussion with Dave, wherever status was mentioned above to describe the pattern state, state should be used.