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

Fix default behavior of nested patterns #1232

Merged
merged 3 commits into from Aug 23, 2020

Conversation

JosefBredereck
Copy link
Contributor

Summary of changes:

Switched the default pattern creation way. For missing .md files there was an issue with nested pattern folders and also variants were not working as expected.

@JosefBredereck JosefBredereck changed the title fix default behavior of nested patterns Fix default behavior of nested patterns Aug 16, 2020
Comment on lines -120 to +121
test.equals(p1.name, '00-atoms-00-global-00-colors-variant');
test.equals(p2.name, '00-atoms-00-global-00-colors-variant-minus');
test.equals(p1.name, '00-atoms-00-global-colors-variant');
test.equals(p2.name, '00-atoms-00-global-colors-variant-minus');
Copy link
Contributor

Choose a reason for hiding this comment

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

So glad to see this going away -- this has always been one of those annoying things with how the filename / URL structure was assembled 👍

Comment on lines +45 to +47
(patternlab &&
patternlab.config &&
patternlab.config.allPatternsAreDeeplyNested)
Copy link
Contributor

Choose a reason for hiding this comment

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

@JosefBredereck I can't wait for when we can switch to using Node 14+ which now supports optional chaining natively, no Babel transpiling required!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I was using patternlab?.config?.allPatternsAreDeeplyNested first, but then it did not work. (Had still V13.x)

But we need to be careful since not everyone uses V14+ now and maybe not for a long time.

@@ -59,3 +59,5 @@ The `deeplyNested` attribute is used to toggle the pattern building behavior and

- **deeplyNested not set or false** - Pattern won't be handled as a deeply nested pattern
- **deeplyNested: true** - Pattern will be handled like mentioned under [Deeper Nesting](#heading-deeper-nesting)

To turn on this behavior globally, just add `"allPatternsAreDeeplyNested": true` to your `patternlab-config.json`.
Copy link
Contributor

Choose a reason for hiding this comment

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

+1 so existing PL installs not already switched to using allPatternsAreDeeplyNested won't run into any file name / path changes; just applies to the new spec we're moving towards.

@sghoweri sghoweri self-requested a review August 23, 2020 15:36
@coveralls
Copy link

coveralls commented Aug 23, 2020

Coverage Status

Coverage decreased (-0.05%) to 74.486% when pulling 62cb282 on fix-variant-files-not-nested into cb7049f on dev.

Copy link
Contributor

@sghoweri sghoweri left a comment

Choose a reason for hiding this comment

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

I'm good with this -- all small sensible changes + only scoped to the latest (new) opt-in allPatternsAreDeeplyNested config option. 👍

Thanks @JosefBredereck!

@sghoweri sghoweri self-requested a review August 23, 2020 15:38
Copy link
Contributor

@sghoweri sghoweri left a comment

Choose a reason for hiding this comment

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

Err - hold on a sec. Seeing something weird with the handlebars deployment

@sghoweri sghoweri self-requested a review August 23, 2020 16:14
Copy link
Contributor

@sghoweri sghoweri left a comment

Choose a reason for hiding this comment

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

Ok - after merging in the reverted code off of dev the handlebars demo is once again working as expected. Phew 😅

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

Successfully merging this pull request may close these issues.

None yet

3 participants