Update pattern discovery and fix "view all" issues #1143

JosefBredereck · 2020-04-12T19:54:46Z

Closes

Issue	Title
#890	[uikit-workshop] View All for subtypes is broken
#1134	Since 5.7.0, PL menu structure breaks if filename and subfolder name match
#633	Ignore files nested 2 levels deep
#660	Unable to generate "View All" pages on flat patterns
#714	Support view all pages in pattern linking
#1016	(errors resulting from) Ensure that patterns in directories greater than 2 levels deep from source are sorted to the next possible parent directory.

Replaces

Issue	Title
#1138	Introduce the ability to configure the nesting level of the pattern directory hierarchy

Summary of changes:

Features

Ability to link view all pages via link.viewall-group-all and link.viewall-group-subgroup

Bugs

Fixed test cases incorrect
Fixed view all pages not generated
Fixed view all page could not be loaded
Fixed pattern ordering
Fixed subfolder patterns have invalid data
Fixed subfolder patterns are not rendered properly
Fixed deeper nesting patterns are ignored
Fixed deeper nesting patterns are not rendered properly

- Fixed test cases incorrect - Fixed view all pages not generated - Fixed view all page could not be loaded - Fixed pattern ordering - Fixed subfolder patterns have invalid data - Fixed subfolder patterns are not rendered properly

… into dev

packages/core/src/lib/object_factory.js

JosefBredereck · 2020-04-12T19:59:21Z

packages/core/src/lib/object_factory.js

@@ -125,7 +117,7 @@ const Pattern = function(relPath, data, patternlab) {
  this.lineageR = [];
  this.lineageRIndex = [];
  this.isPseudoPattern = false;
-  this.order = Number.MAX_SAFE_INTEGER;
+  this.order = 0;


Patterns should be sorted after groups and before view all page of that group.

packages/core/src/lib/object_factory.js

packages/core/test/object_factory_tests.js

packages/core/test/ui_builder_tests.js

JosefBredereck · 2020-04-12T20:03:25Z

packages/core/test/ui_builder_tests.js

@@ -222,13 +223,19 @@ tap.test('groupPatterns - creates pattern groups correctly', function(test) {
    'patternType1-white'
  );

+  // Flat patterns
  test.equals(


Additional test for flat patterns

packages/core/test/ui_builder_tests.js

packages/core/src/lib/ui_builder.js

JosefBredereck · 2020-04-12T20:14:02Z

@bmuenzenmeyer I have also added some comments in here 1143/files for lines or blocks where it seems to be necessary. Feel free to add additional comments.

After hours and hours of debugging, I hope I could resolve all issues provided here. 💯

JosefBredereck

Found additional features and issues to resolve

packages/core/patternlab-config.json

bmuenzenmeyer · 2020-04-14T12:10:51Z

@JosefBredereck excited to look at this closer - but I need a day or so more

JosefBredereck · 2020-04-14T12:12:21Z

Take your time. This should be checked completely and its not just one line of code I changed.

coreylafferty · 2020-04-16T18:54:51Z

@JosefBredereck I gave this a quick test and it does appear to fix #1134. Thank you! I'm not sure if it's an intentional change or not, but I did notice that any categories with nested items now appear at the top of the list before those without children. Just thought I'd draw attention to it in case that's not the intended behavior.

JosefBredereck · 2020-04-16T19:31:14Z

@JosefBredereck I gave this a quick test and it does appear to fix #1134. Thank you! I'm not sure if it's an intentional change or not, but I did notice that any categories with nested items now appear at the top of the list before those without children. Just thought I'd draw attention to it in case that's not the intended behavior.

I will test that again explicit, thanks for the hint. 👍

JosefBredereck · 2020-04-16T20:06:31Z

@coreylafferty that's what I get and it looks correct to me. When you add a pattern in the same folder as the pattern name the folder will get removed. The folders are for better structuring in your folder architecture. See #992 - Also if you add something like atoms/atoms.twight, that wouldn't make much sense.

And when I click on broken item 1 it's also correct.

coreylafferty · 2020-04-16T20:53:46Z

@JosefBredereck I just tested it again and I am seeing some odd behavior. Below is a screenshot of our theme with your changes applied.

I'd expected to see 'Article' come before 'Button', but instead all of the items with children are grouped at the top of the list followed by single items. That may be the intention so maybe that doesn't need to change... but there is a bigger problem.

More troubling is that now 'Button' is both a folder and a single item. I would expect that the single 'Button' item would be in the 'Button' folder with the variants (e.g., Danger Button, Large Button, etc.).

The way we organize our files is according to BEM class names, so that's why this is important. For example, here is how Article and Button are structured in our folders:

patterns/04-components/article/article.twig
patterns/04-components/button/button.twig
patterns/04-components/button/button--danger/button--danger.twig
patterns/04-components/button/button--large/button--large.twig

For reference, here's what our theme looks like using Pattern Lab version 5.4.0 (prior to the change that broke things for us)

JosefBredereck · 2020-04-16T21:01:37Z

Let me think about that until tomorrow. Maybe I can get a good solution for that. 🤔 Very good case by the way.

coreylafferty · 2020-04-16T21:26:28Z

Let me think about that until tomorrow. Maybe I can get a good solution for that. 🤔 Very good case by the way.

Thank you! Please let me know if I can give you any more details or test anything.

coreylafferty · 2020-05-20T14:39:09Z

@bmuenzenmeyer @JosefBredereck Does Josef's answer above point towards next steps and/or are specific changes still requested to this PR? Hoping to see this accepted (when it's ready) as the fixes here will unblock us from upgrading PL in our theme. For what it's worth, I agree with with @bmuenzenmeyer's assessment of the subfolder variable... I prefer that name (subfolder over subbed) and think when it's true (or not set), items should show in a subfolder.

Condition Outcome

subfolder not set Buttons shows up under Buttons Menu

subfolder: true Buttons shows up under Buttons Menu

subfolder: false Buttons shows up as a "flat pattern"

JosefBredereck · 2020-05-20T14:41:39Z

From my point of view there was nothing left so far.

JosefBredereck · 2020-07-01T17:34:46Z

Updated from dev.
I made the flat patterns on view all pages and view all pages for only flat patterns optional. So we don't have a breaking change and if someone wants to turn it on the person is now able to.

See https://patternlab.io/docs/overview-of-patterns/#heading-deeper-nesting

JosefBredereck · 2020-07-01T18:32:27Z

For what it's worth, I read it as a noun, so the behavior to me seems like the exact opposite of what it should be.

I expect: (when buttons.hbs exist within buttons/)

Condition Outcome

subfolder not set Buttons shows up under Buttons Menu

subfolder: true Buttons shows up under Buttons Menu

subfolder: false Buttons shows up as a "flat pattern"

I changed this to a more specific name deeplyNested and also updated the documentation regarding this change. So everything will be aligned with Pattern Overview Deeper Nesting. And we won't have a breaking change again. So we should be aligned with all the discussions going on in this thread.

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

sghoweri · 2020-07-30T19:19:09Z

For what it's worth, I read it as a noun, so the behavior to me seems like the exact opposite of what it should be.
I expect: (when buttons.hbs exist within buttons/)

Condition
Outcome

subfolder not set
Buttons shows up under Buttons Menu

subfolder: true
Buttons shows up under Buttons Menu

subfolder: false
Buttons shows up as a "flat pattern"

I changed this to a more specific name deeplyNested and also updated the documentation regarding this change. So everything will be aligned with Pattern Overview Deeper Nesting. And we won't have a breaking change again. So we should be aligned with all the discussions going on in this thread.

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

Hey @JosefBredereck @coreylafferty so sorry for being a bit out of the loop on this -- this good to get merged down and released, yes?

JosefBredereck · 2020-07-30T19:32:53Z

From my point of view, yes. I also used this branch in a production application already.

I hope the docs are enough and grammatically correct.

coreylafferty · 2020-07-31T18:46:44Z

@sghoweri @JosefBredereck I won't pretend to understand all the changes here, but in my testing this solves the nesting problem I'd described in #1134 and the nesting behaves as I'd expect. Setting the DeeplyNested value in the MD files does move the pattern in or out of a subfolder, so that's working as described, with the default, undefined condition being how PL has traditionally handled the nesting.

I'll admit I don't fully understand what the name 'DeeplyNested' is meant to indicate since at first I expected it to behave the opposite than it does -- with the 'false' condition being the one that flattens the item in the menu structure, placing it outside its own folder (which currently is the True condition)... but since the default/false is to have the menu behave as I'd expect, I don't see the name being a big issue.

Changes since original PR feedback submitted

sghoweri

Let's get this shipped 🚀 -- thanks again for the added docs and ton of new tests added with this. 👍

sghoweri · 2020-08-09T15:51:12Z

PR was released with v5.12.0

…lab#1202

Update pattern discovery and fix "view all" issues

JosefBredereck added 3 commits April 12, 2020 02:20

Update object_factory and view all page behavior

2237301

Various changes to file and pattern creation

edc4b0c

- Fixed test cases incorrect - Fixed view all pages not generated - Fixed view all page could not be loaded - Fixed pattern ordering - Fixed subfolder patterns have invalid data - Fixed subfolder patterns are not rendered properly

Merge branch 'dev' of https://github.com/JosefBredereck/patternlab-node…

f9ca91e

… into dev

JosefBredereck requested a review from sghoweri as a code owner April 12, 2020 19:54