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

Discussion & Vote: defaultPattern config option #13

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

Discussion & Vote: defaultPattern config option #13

bmuenzenmeyer opened this issue Jun 1, 2016 · 12 comments

Comments

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Jun 1, 2016

As a design system creator using Pattern Lab, I want the first glimpse of a deployed version of Pattern Lab to be a single pattern of my choosing, so that I can welcome consumers of the design system, and have the opportunity to better explain the system to them with a landing page of sorts.


This feature is proposed from the PL/Node team, as already implemented in this PR. You can see at the time it was a conscious deviation from shared assets, and as such, is a regression of sorts as we integrate into the shared asset environment. A note: the PR is included here for functional understanding only, and is not necessarily the suggested implementation. I think that features should be up or down voted on their functional merit during these discussions, not the particular code that might serve as a reference.

  • @geoffp has shown me how Target uses this to effectively introduce their Pattern Lab. Geoff, please elaborate or weigh in if I've misrepresented anything
  • Before this feature was in place, a user had to share a query-stringed url as the "entry point" of the system, or create as the very first pattern in the tree some sort of 00-start or 00-welcome file.

Timeline for this change is, in my opinion, not critical to 2.0.0 release.

Tagging core and shared assets due to this being an additional configuration option set in core that affects frontend.

This vote will close at noon central on June 15, 2016 or once both the PHP and Node representatives have voted.

/cc @pattern-lab/voting-members

@geoffp
Copy link

geoffp commented Jun 1, 2016

Yes -- we use this to do nice explanatory landing pages to offer orientation and documentation. It's not pretty right now, but something like this:

pattern lab - pages-lab-home 2016-06-01 09-25-59

@bradfrost
Copy link
Member

We've had other requests to make the homepage of PL something other than View All. I think it's a good option.

One of the other things @dmolsen and I were discussing a while back was the ability to add links to the top menu dropdown, so that people could link out to code style guides, voice and tone, repos, and other resources.

@dmolsen
Copy link
Member

dmolsen commented Jun 1, 2016

I see the value but trying to nail down the mechanics of this based on the PR.

System Input

The user sets this by updating a defaultPattern option in the config. It should be formatted as a pattern partial. For example, pages-intro or similar. By default the config option should be set to all.

System Output

It should be passed to the viewer via a config option. It should probably work something like:

// in styleguide.js per the PR, missing option in PL/PHP might get set to false by default
var patternName = ((config.defaultPattern !== undefined) && (typeof config.defaultPattern === 'string') && (config.defaultPattern.trim().length > 0)) ? config.defaultPattern : 'all';

Questions

At least two additional complications:

  • if a user sets defaultPattern to baz so no file name is found should we use the default styleguide path?
  • if a user puts in ?pattern=baz should the defaultPattern be used or all? If defaultPattern also fails do we fall all the way back to the default styleguide path?

@bmuenzenmeyer
Copy link
Member Author

@dmolsen - responses

if a user sets defaultPattern to baz so no file name is found should we use the default styleguide path?

Hmm. I hadn't thought of how this behaves, or should behave. @geoffp what happens if you delete your defaultPattern out from under the config today? Identifying this scenario and reverting back to the normal behavior would be important, if not emitting a console.warning() too.

if a user puts in ?pattern=baz should the defaultPattern be used or all?

baz should be used. query string parameters take precedence. defaultPattern is limited to altering behavior where, lacking any other input, all is used by the viewer. This feature should not override the ALL button's behavior on the ish controlba, howerver.

If defaultPattern also fails do we fall all the way back to the default styleguide path?

same as first comment

@bradfrost - response

One of the other things @dmolsen and I were discussing a while back was the ability to add links to the top menu dropdown, so that people could link out to code style guides, voice and tone, repos, and other resources.

should that be a separate feature request, judged on its own merits?

@dmolsen
Copy link
Member

dmolsen commented Jun 3, 2016

baz should be used.

baz was meant to illustrate an invalid pattern name. Right now an invalid pattern name defaults to the "all" view.

should that be a separate feature request, judged on its own merits?

While we could go that way I'm also open to making this a plugin instead of worrying about this as part of spec/core. We're supposed to be pushing to a more modular design rather than tacking on specific features. The only thing from a spec side I could see would be:

  • allow a user pass a custom config var from the PL config to the front-end? PL/PHP currently whitelists these. Maybe that's overkill.
  • where do we add DOM hooks to the viewer mark-up and do we have a naming convention?
  • where should events fire during the viewer JS and do we have a naming convention?

This way a plugin could read in the list of links from a config var and add mark-up to an available node based on an event firing.

I think those are two separate discussions split on config vs. DOM hooks and events. Frankly, the latter might not even be spec because it doesn't really address an input/output for Core but rather just the viewer. I would welcome feedback though. I have an idea about "panels" floating around in my head. I'll put that and the DOM hooks/events up for discussion mid-next week.

@dmolsen
Copy link
Member

dmolsen commented Jun 10, 2016

To summarize for a vote:

System Input

The user sets this by updating a defaultPattern option in the config. It should be formatted as a pattern partial. For example, pages-intro or similar. By default the config option should be set to all.

System Output

It should be passed to the viewer via a config option. It should probably work something like:

// in styleguide.js per the PR, missing option in PL/PHP might get set to false by default
var patternName = ((config.defaultPattern !== undefined) && (typeof config.defaultPattern === 'string') && (config.defaultPattern.trim().length > 0)) ? config.defaultPattern : 'all';

Default actions

If a user sets defaultPattern to baz so no file name is found PL will use the default "all" view.

If a the defaultPattern is set to a real pattern and a user puts in ?pattern=baz which doesn't find a pattern then PL will use defaultPattern.

The only way to get to a defaultPattern is to reload the entire page.

@bmuenzenmeyer
Copy link
Member Author

The only way to get to a defaultPattern is to reload the entire page.

Does this mean it would be excluded from the navigation?

@dmolsen
Copy link
Member

dmolsen commented Jun 12, 2016

@bmuenzenmeyer -

From the PR you linked to and screenshot above it looks like the default pattern was never in the nav. You also mention:

This feature should not override the ALL button's behavior on the ish controlba, howerver

So between those it seems like the only way to get to the default pattern is a reload. I'm open to ideas.

@geoffp
Copy link

geoffp commented Jun 13, 2016

We put orientation text and git build date, commit, and branch on this page. In our implementation of this, the pattern is present in the nav, in the Pages menu, but the way we use it, I don't think anybody minds doing a refresh to get back to it if they need to. For our purposes, this is acceptable. It would feel cleaner to have some more recognizable path back. I could see adding a "Home" button for this case, but we're already hard up for real estate up there in the nav bar and I personally would hesitate to add another button.

Maybe there's some value in supporting a "home button" once somebody has an alternative UI kit with a roomier nav strategy.

@bmuenzenmeyer
Copy link
Member Author

If Geoff has observed users not need to go to the content of their defaultPattern often, I suppose it being excluded from the nav is acceptable. If I remember correctly, it was a bit odd to have this "homepage" in some arbitrary location inside the pages menu - putting into question from a simple eye-scan if that was a legitimate pattern or not..

So..
For the purposes of this spec enhancement, I am okay with nav exclusion of whichever patternPartial is defined in defaultPattern, but only when defaultPattern is not all.

Maybe there's some value in supporting a "home button" once somebody has an alternative UI kit with a roomier nav strategy.

I do think there's value in a homepage of sorts as you mention and Brad alluded to before. A page like https://www.lightningdesignsystem.com/ really gives Salesforce an intentionally designed landing experience, but I think adding a "Home" link should be another spec discussion.

Unless anything else need be discussed, I am ready to vote, taking in Dave's summary and my comment above.

Vote: 👍

@dmolsen
Copy link
Member

dmolsen commented Jun 14, 2016

For the purposes of keeping this moving we'll implement what I've summarized.

👍

@dmolsen
Copy link
Member

dmolsen commented Jun 19, 2016

This has been implemented and will be hitting various dev branches on the repos this afternoon.

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

4 participants