Skip to content
This repository has been archived by the owner on May 1, 2020. It is now read-only.

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

Closed
bmuenzenmeyer opened this issue Jun 1, 2016 · 19 comments

Comments

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Jun 1, 2016

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:

  • reduce fatigue with yet more regexes
  • I also thought it'd be simpler to change states in one place rather than have to rename files- an action, that, in my opinion, always feels costly from a UX perspective (just clicking through and editing) and results in a mess for version control systems.

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

  • parse status out of a pattern's .md Front Matter and map to a pattern's patternState proprerty
  • parse order out of a pattern's .md Front Matter and use to override any file order prior to rendering
  • parse hidden out of a pattern's .md Front Matter and use to hide pattern from all direct individual or subtype rendering similar to _underscoredPattern.mustache today. These patterns should still be include-able as partials
  • parse excludeFromStyleguide (or a better name) out of a pattern's .md Front Matter and use to omit pattern from all rendering
  • parse tags out of a pattern's .md Front Matter and ... ??? (available in a future search enhancement?)
  • parse additional keys ouf of a pattern's .md Front Matter and display in {{patternDescAdditions}}

/cc @pattern-lab/voting-members

@bradfrost
Copy link
Member

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
Copy link
Member Author

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 🎯

@dmolsen
Copy link
Member

dmolsen commented Jun 1, 2016

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

@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
Copy link
Member

dmolsen commented Jun 1, 2016

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 [email protected] 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
Copy link
Member Author

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...

  • create a vote issue to reference this discussion regarding status
  • create a discussion or discussion & vote issue to discuss order

?

@dmolsen
Copy link
Member

dmolsen commented Jun 2, 2016

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

  • status
  • tags
  • order
  • hidden

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
Copy link
Member Author

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

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.

@dmolsen
Copy link
Member

dmolsen commented Jun 2, 2016

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 [email protected] 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
Copy link
Member Author

bmuenzenmeyer commented Jun 3, 2016

@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 #16 (comment) questions regarding hidden and the styleguideExclude option.

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

@bmuenzenmeyer bmuenzenmeyer changed the title Discussion: Pattern State Declaration Discussion and Vote: Pattern State Declaration, and other Pattern Metadata Jun 3, 2016
@dmolsen
Copy link
Member

dmolsen commented Jun 3, 2016

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

dmolsen commented Jun 10, 2016

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
Copy link
Member Author

bmuenzenmeyer commented Jun 11, 2016

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

dmolsen commented Jun 14, 2016

@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
Copy link
Member Author

bmuenzenmeyer commented Jun 14, 2016

@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
Copy link
Member

dmolsen commented Jun 15, 2016

👍

1 similar comment
@bmuenzenmeyer
Copy link
Member Author

👍

@bmuenzenmeyer
Copy link
Member Author

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

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

No branches or pull requests

3 participants