Skip to content

Conversation

@sghoweri
Copy link
Contributor

Checks to make sure the code panel-related <script> tag contains data before attempting to parse expected JSON.

Partial fix to #761 as this should at least help prevent the current batch of JS errors from getting thrown till the larger data structure changes that caused this issue are identified and a fix rolled up.

Related to #905 as Twig allows us to massage the data available at the template level to work around whatever data changes happened to cause this bug in the first place.

…data before attempting to parse expected JSON. Partial fix to pattern-lab#761 as this should at least help prevent the current batch of JS errors from getting thrown
@bmuenzenmeyer bmuenzenmeyer self-requested a review July 24, 2018 16:03
@bmuenzenmeyer bmuenzenmeyer self-assigned this Jul 24, 2018
@bmuenzenmeyer
Copy link
Member

Love this - reviewing soon!

Copy link
Member

@bmuenzenmeyer bmuenzenmeyer left a comment

Choose a reason for hiding this comment

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

Something is either wrong with this or how I am testing it.

I couldn't get the uikit to compile without changing a few things (const back to var), defining var patternMarkupEl

and then I couldn't reliably get the issue to occur prior to the fix being applied.

Can you run npm run watch and confirm it builds for you?

@sghoweri
Copy link
Contributor Author

@bmuenzenmeyer hmmmm lemme take a look and I'll get back to you on this one!

…abel just yet.. update Prettier config to ignore Uikit's JavaScript for the time being
@sghoweri
Copy link
Contributor Author

sghoweri commented Jul 28, 2018

ok I think I figured out the issue you ran into @bmuenzenmeyer after walking through this step by step -- basically, Prettier is currently set to lint / auto-format all JS in the monorepo which, incidentally, includes the very-much-in-need-of-refactoring JavaScript in Pattern Lab's UIkit.

That would explain why this change originally worked / built without issues on my end initially but I ran into the same problems you did after pulling my branch back down and re-running.

I've went back in and (for the time being at least) updated the root .prettierignore config to ignore the JS files in the uikit-workshop/src/js folder + updated that const declaration in my changes here back to var and everything ran fine for me locally -- testing instructions (for your reference -- and for my own sanity check) are below:


  1. Check out a copy of the repo this PR is based off of + install deps
git clone https://github.com/sghoweri/patternlab-node.git patternlab-node-pr # or whatever folder
cd patternlab-node-pr
git checkout fix/partial-code-panel-toggle-fix

npm install && npm run bootstrap
cd packages/edition-node

## manually install starterkit-mustache-demo for testing this out -- CLI install not working for me :-/
npm install @pattern-lab/starterkit-mustache-demo
  1. Update patternlab-config.json to point at the Mustache Starterkit Demo assets + Switch to default theme layout to horizontal by default (so you don't have to resize your browser to recreate the issue here):
{
  "cacheBust": true,
  "cleanPublic": true,
  "defaultPattern": "all",
  "defaultShowPatternInfo": false,
  "ishControlsHide": {
    "s": false,
    "m": false,
    "l": false,
    "full": false,
    "random": false,
    "disco": false,
    "hay": true,
    "mqs": false,
    "find": false,
    "views-all": false,
    "views-annotations": false,
    "views-code": false,
    "views-new": false,
    "tools-all": false,
    "tools-docs": false
  },
  "ishViewportRange": {
    "s": [240, 500],
    "m": [500, 800],
    "l": [800, 2600]
  },
  "logLevel": "info",
  "outputFileSuffixes": {
    "rendered": ".rendered",
    "rawTemplate": "",
    "markupOnly": ".markup-only"
  },
  "paths": {
    "source": {
      "root": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist",
      "patterns": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/_patterns/",
      "data": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/_data/",
      "meta": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/_meta/",
      "annotations": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/_annotations/",
      "styleguide": "dist/",
      "patternlabFiles": {
        "general-header":
          "views/partials/general-header.mustache",
        "general-footer":
          "views/partials/general-footer.mustache",
        "patternSection":
          "views/partials/patternSection.mustache",
        "patternSectionSubtype":
          "views/partials/patternSectionSubtype.mustache",
        "viewall":
          "views/viewall.mustache"
      },
      "js": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/js",
      "images": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/images",
      "fonts": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/fonts",
      "css": "./node_modules/@pattern-lab/starterkit-mustache-demo/dist/css"
    },
    "public": {
      "root": "public/",
      "patterns": "public/patterns/",
      "data": "public/styleguide/data/",
      "annotations": "public/annotations/",
      "styleguide": "public/styleguide/",
      "js": "public/js",
      "images": "public/images",
      "fonts": "public/fonts",
      "css": "public/css"
    }
  },
  "patternExtension": "mustache",
  "patternStateCascade": ["inprogress", "inreview", "complete"],
  "patternExportDirectory": "./pattern_exports/",
  "patternExportPatternPartials": [],
  "serverOptions": {
    "wait": 1000
  },
  "starterkitSubDir": "dist",
  "styleGuideExcludes": [],
  "theme": {
    "color": "light",
    "density": "compact",
    "layout": "horizontal"
  },
  "uikits": [
    {
      "name": "uikit-workshop",
      "outputDir": "",
      "enabled": true,
      "excludedPatternStates": [],
      "excludedTags": []
    }
  ]
}
  1. Confirm that clicking on the code panel toggle button does not throw a JS error in the console
npm run pl:serve

Before

image

After

image

@bmuenzenmeyer bmuenzenmeyer changed the base branch from master to dev August 3, 2018 14:03
@bmuenzenmeyer
Copy link
Member

I no longer see the error. I think it's a separate issue that the panel does not open in the view-all screens.

@bmuenzenmeyer bmuenzenmeyer merged commit 6921ab5 into pattern-lab:dev Aug 3, 2018
@bmuenzenmeyer
Copy link
Member

Thanks for the stellar work and testing directions, by the way!

antonia-rose pushed a commit to quelltexterin/nemo-uikit-workshop that referenced this pull request Apr 12, 2023
…el-toggle-fix

Partial Fix for Code Panel Toggle JS Errors on Master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants