Variants - inheriting data from the base

[legostud]

I created an Organism that includes an array of a single molecule.
http://mayflower.digital.mass.gov/?p=organisms-contact-list

I then created a variant of that Pattern using the contact-list~single-item.json method to show how the component should be rendered if there is only one molecule in the array.
http://mayflower.digital.mass.gov/?p=organisms-contact-list-single-item

After upgrading our site to the Drupal Pattern Lab (massgov/mayflower#488), the Variant began showing two items instead of one. The first item was being displayed correctly, but the second item, which shouldn't have been there, was coming from the base json file.

I'm not sure why this is happening, but it seems like a bug.

{
  "contactList": {
    "compHeading": {
      "title": "Contacts",
      "sub": "",
      "color": "",
      "id": "contacts",
      "centered": ""
    },

    "contacts":[{
      "accordion": false,
      "isExpanded": false,
      "subTitle": {
        "href":"",
        "text": "Department of Conservation and Recreation"
      },
      "groups": [{
        "icon": "@atoms/05-icons/svg-phone.twig",
        "name": "Phone",
        "items": [{
          "type": "phone",
          "property": "",
          "label":"Main:",
          "link": {
            "href": "+14134994262",
            "text": "(413) 499-4262",
            "info": "",
            "property": ""
          },
          "details":"This line is open from 9:00 a.m. - 5:00 p.m., M-F"
        },{
          "type": "phone",
          "property": "",
          "label":"Toll Free:",
          "link":{
            "href": "+18004324321",
            "text": "(800) 432-4321",
            "info": "",
            "property": ""
          },
          "details":"This line is open from 9:00 a.m. - 5:00 p.m., M-F"
        },{
          "type": "phone",
          "property": "",
          "label":"TTY:",
          "link": {
            "href":"+16175557777",
            "text": "(617) 555-7777",
            "info": "",
            "property": ""
          },
          "details":"For those who need accessible services"
        },{
          "type": "phone",
          "property": "",
          "label":"Voice Relay:",
          "link":{
            "href":"16175555255",
            "text": "(617) 555-5255",
            "info": "",
            "property": ""
          },
          "details":""
        }]
      },{
        "icon": "@atoms/05-icons/svg-laptop.twig",
        "name": "Online",
        "hidden": "",
        "items": [{
          "type": "email",
          "property": "",
          "label":"Email:",
          "link": {
            "href":"[email protected]",
            "text": "[email protected]",
            "info": "",
            "property": ""
          },
          "details":""
        },{
          "type": "online",
          "property": "",
          "label":"Web:",
          "link": {
            "href":"#",
            "text": "Contact Form",
            "info": "",
            "property": ""
          },
          "details":""
        }]
      },{
        "icon": "@atoms/05-icons/svg-fax-icon.twig",
        "name": "Fax",
        "items": [{
          "type": "fax",
          "property": "",
          "label":"Main:",
          "link": {
            "href":"+14134994262",
            "text": "(413) 499-4262",
            "info": "",
            "property": ""
          },
          "details":"Please allow 24 hours for a response"
        }]
      },{
        "icon": "@atoms/05-icons/svg-marker.twig",
        "name": "Address",
        "items": [{
          "type": "address",
          "property": "",
          "label": "",
          "address": "555 Park Drive\nBoston, MA 02231",
          "link": {
            "href": "https://www.google.com/maps/place/555+Park+Dr,+Boston,+MA+02215",
            "text": "directions",
            "info": "",
            "property": ""
          },
          "details": ""
        }]
      }]
    }]
  }
}

[sghoweri]

Hey @legostud - so after doing quite a bit of digging (and a whole lot of experimenting with the logic behind how the psuedo-pattern data gets merged together in the vendor/pattern-lab/core/src/PatternLab/PatternData/Rules/PseudoPatternRule.php file) I think I better understand the problem here... but I'm not sure I necessarily have a "solution" per say.

Lemme try to break this down a bit.

1. Regarding this quirk showing up w/ the PR from a couple weeks ago updating Mayflower to the Drupal Pattern Lab Fork:

From what I can tell, this issue with some of the data getting duplicated in the pseudo pattern stems from this commit from @dmolsen from a little over a year ago that changed the default data merging behavior for pseudo-patterns in Pattern Lab from a simple array_merge to a array_replace_recursive merge.

Sure enough, if I revert those two lines locally, the single item variant renders how I believe you'd be expecting it to:

2. That said, this brings up the important question, what's the correct behavior here?

Let's pretend for a moment that this single item variant didn't/shouldn't want to manually re-include the "Contacts" title by default (after all, that's sort of the whole point of having inherited data).

Rather than setting the contactList.compHeading.title value to an empty string, let's assume we just don't include that value in our json file and re-generate Pattern Lab.

contactList": {
    "example-prop": "contact list",
    "viewSpecific": false,
    "compHeading": {
      "sub": "",
      "color": "",
      "id": "contacts",
      "centered": ""
    },
    ...
}

Instead of inheriting the "Contact Us" text from the original pattern (or some other data property we'd want to trickle down into variants so we don't need to duplicate the data structure over and over again, instead the title gets overwritten by the other compHeading data values and we're left without a title...

vs. the update from July of last year which replaced array_merge with array_replace_recursive

which properly inherit the default title set in the base pattern. If I'd have to guess, this is probably why the data across the board in PL is replaced recursively vs merged.

So.... what's the correct behavior? ¯_(ツ)_/¯

3. All that said, I think there's a couple options we could go from here.

My personal opinion would be to go in one of two different directions:

Option 1: Keep the default merging behavior, but take a slightly different approach to generating variants.

@EvanLovely had some awesome slides during his Drupalcon Baltomore 2017 presentation on working with Pinterest on their component library and had this one slide in particular that really stood out:

Essentially, each pattern should have a vanilla pristine underscored version that both PL and Drupal reference and pass in the data per usual via separate twig file variations.

So for this example, you could simply have a _contact-list.twig template that gets included or extended via a separate contact-list--single-item.twig file to get all the benefits of the array_merge + array_replace_recursive:

{% extends "@organisms/by-author/_contact-list.twig" %}

OR

{% include "@organisms/by-author/_contact-list.twig" %}

Option 2: Allow for Default PL Rules to Be Configurable

Alternatively, if we went down this route, we could allow PL users to pick and choose which is the right behavior for them (while still providing an out of the box default set of rules to start off with).

---

Side note: I'm sad that the slides for Evan's presentation didn't get included in the YouTube video. A PDF of the slides would be amazing....

CC @EvanLovely, @aleksip, or @jesconstantine -- any thoughts?

[legostud]

@sghoweri - In your example of omitting "title", the pattern is still rendering (see the green line) just without any text. All of the html is still there.

In the example I'm showing, the second item is an accordion that expands to show another element.

The question I still have is that I have reset the "contactList.contacts" variable from having two values to having one value in it array, as such it should render one element. I could accept it rendering two elements if I omitted the contacts variable, because I then wouldn't be overriding the default contacts variable.

[EvanLovely]

Salem, I'm on vacation this week, but I wanted to drop in the link to my slides from the presentation. I'm super bummed they messed up the recording and plan to fix it myself eventually.

http://www.evanlovely.com/presentations/drupalcon-2017-pinterest-pattern-lab/DrupalCon_2017_-_Pinterest_Pattern_Lab.pdf

[legostud]

Those slides provided some insight into this problem. Base on what I'm seeing in Evans slides, the pattern for creating variants is different than how I've been doing them. In these examples, all of the data passed is done using 'with' so a variant is created as a new twig file with a different data object passed down.

For my project, I'm using pseudo patterns, which uses multiple json files for one twig file.
http://patternlab.io/docs/pattern-pseudo-patterns.html

[legostud]

found another broken pattern (two column variant). Similar issue with the array being extended.

[sghoweri]

Hey thanks @EvanLovely - enjoy your time off!