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

Feature: Add The Ability To Hide Viewall Links #1034

Merged
merged 15 commits into from
Nov 26, 2019

Conversation

sghoweri
Copy link
Contributor

@sghoweri sghoweri commented Aug 1, 2019

Summary

Adds the ability to hide any viewall links that would normally show up in the Pattern Lab navigation.

Details

Updates the Webpack build config + .patternlabrc config to support a new noViewAll config option when customizing Pattern Lab's UI.

Before vs After

image

image

How To Test

  1. Pull down this branch and install NPM dependencies by running npm run setup.
  2. Go to the packages/edition-twig folder and uncomment line 5 in the example .patternlabrc config file.
module.exports = {
  buildDir: __dirname + '/public',
  wwwDir: __dirname + '/public/',
  publicPath: '/public/styleguide/',
  // noViewAll: true, // <!-- uncomment this line
};
  1. Rebuild Pattern Lab's UI by running npm run build:uikit from the same packages/edition-twig folder
  2. Confirm the regenerated UIKit is rendering properly by now running npm run serve

image

@jryanconklin
Copy link
Contributor

Just a note to say I'd really love to have this.

@bmuenzenmeyer
Copy link
Member

@sghoweri can i switch this target to dev ?

@sghoweri
Copy link
Contributor Author

@bmuenzenmeyer go for it!

@sghoweri sghoweri changed the base branch from feature/uikit-refactor--batched-updates to dev August 20, 2019 06:19
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.

playing catchup on this feature - it is not accessible from a patternlab-config.json entry, correct? ony at build time from the uikit?

EDIT:

looking at that edition for the first time - adding build:uikit is an interesting end-user customization feature! 👍

$INIT_CWD is not cross-platform safe - going to try cross-env

@@ -2,4 +2,5 @@

module.exports = {
// buildDir: __dirname + '/www/pattern-lab',
noViewAll: true, // uncomment to disable displaying viewAll links in Pattern Lab
Copy link
Member

Choose a reason for hiding this comment

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

this comment doesnt make sense, as it's enabled, not commented

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Derp — I think I was thinking we’d want this option to be disabled by default (hence the comment) but I probably I uncommented that when I was putting together my PR screenshot 🤦‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

bump

@sghoweri
Copy link
Contributor Author

playing catchup on this feature - it is not accessible from a patternlab-config.json entry, correct? ony at build time from the uikit?

EDIT:

looking at that edition for the first time - adding build:uikit is an interesting end-user customization feature! 👍

$INIT_CWD is not cross-platform safe - going to try cross-env

@bmuenzenmeyer yeah customizing / extending PL is an exciting area I’m hoping to spend some time planning out / documenting / building up!

At a high level, I’m thinking about 3 overall ways PL’s UI could get customized:

  1. Via runtime / JSON config (patternlab-config.json) not requiring the UI to recompile
  2. Via patternlabrc.js (requiring UI to get rebuilt, say via the CLI)
  3. And declaratively via customizing the web component HTML getting used

@bmuenzenmeyer
Copy link
Member

@sghoweri can you change this to use cross-env? otherwise I can try quick

it'd be nice to batch this in to the release (I hope we release today)

@sghoweri
Copy link
Contributor Author

@bmuenzenmeyer I can check this out now - no prob

@bmuenzenmeyer
Copy link
Member

I am going to try to get this working on Windows then.

@bmuenzenmeyer
Copy link
Member

Also like to get this into development-edition-engine-handlebars for now

@bmuenzenmeyer
Copy link
Member

trying to work through this

Error: ENOENT: no such file or directory, open 'D:\src\patternlab-node\packages\development-edition-engine-handlebars\node_modules\@pattern-lab\uikit-workshop\$INIT_CWD\.patternlabrc.js'

@bmuenzenmeyer
Copy link
Member

this works!?

"build:uikit": "npm run build --prefix node_modules/@pattern-lab/uikit-workshop -- --patternlabrc ../../../.patternlabrc.js",
```

@bmuenzenmeyer
Copy link
Member

I am still seeing the pattern type view all, but sub type appears gone

image

does this seem right @sghoweri

@sghoweri
Copy link
Contributor Author

Hmmm

@bmuenzenmeyer
Copy link
Member

changing noViewAll seems to have no effect for me within my codebase

@bmuenzenmeyer
Copy link
Member

pushed https://github.com/pattern-lab/patternlab-node/tree/1034-test

npm run setup
cd packages/development-edition-engine-handlebars
change `noViewAll` within `.patternlabrc.js`
npm run build:uikit
npm run pl:serve

@bmuenzenmeyer
Copy link
Member

@sghoweri game to release dev as is and figure this out as a fast-follower?

@sghoweri
Copy link
Contributor Author

@sghoweri game to release dev as is and figure this out as a fast-follower?

@bmuenzenmeyer I'm good with that!

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Oct 22, 2019

@sghoweri just checking in on this - very excited for the functionality but i want to ensure it works in both Twig and Handlebars editions before we move forward. will try again after a dev upstream merge sometime soon

@bmuenzenmeyer
Copy link
Member

once this merge conflict is resolved, I am hoping that #1098 will immensely help iterate on this solution

@bmuenzenmeyer
Copy link
Member

@bmuenzenmeyer
Copy link
Member

i've spent a fair amount of time with this and cannot get it to work - i wonder now that we have the build-tools.js file if parts of that could be used.

@sghoweri you've also expressed concern that this might not be the best approach. thoughts on making this a runtime config option?

@sghoweri
Copy link
Contributor Author

i've spent a fair amount of time with this and cannot get it to work - i wonder now that we have the build-tools.js file if parts of that could be used.

@sghoweri you've also expressed concern that this might not be the best approach. thoughts on making this a runtime config option?

@bmuenzenmeyer no worries, appreciate you taking a look at this.

I'm looking this over this morning (probably trying to switch to using a runtime config out of simplicity); I'll see what I can do!

@bmuenzenmeyer
Copy link
Member

It's entirely possible I'm either doing something wrong (though I was porting the twig edition code directly) or the is a simple cross platform fix... but I haven't found it yet.

Thanks for plugging away on this with me!

@sghoweri
Copy link
Contributor Author

@bmuenzenmeyer this should be good to go! Updated the PR description + adjusted some stuff internally to now have this be based off of the runtime config vs having to recompile the UI in order to enable / disable this.

@bmuenzenmeyer
Copy link
Member

will give this a go later this weekend @sghoweri - thanks!

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.

This is working 💃

Ideally, this new config item is added to all edition patternlab-config.json files too.

When features like this are released, we should really document them. I don't think there is any patternlab.io documentation on the theme config. I will add a story if there isn't one. @bradfrost mentioned content being a good next step.

publicPath: '/public/styleguide/',
// target the UIKit installed / symlinked under node_modules
buildDir: __dirname + '/node_modules/@pattern-lab/uikit-workshop/dist',
// noViewAll: true,
Copy link
Member

Choose a reason for hiding this comment

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

I think this can be removed

@bmuenzenmeyer
Copy link
Member

I don't think there is any patternlab.io documentation on the theme config.

I was wrong - we did add it here https://patternlab.io/docs/advanced-config-options.html but it could certainly be improved with our rewrite / refresh

@sghoweri sghoweri mentioned this pull request Nov 24, 2019
@bmuenzenmeyer bmuenzenmeyer merged commit 2dc87bd into pattern-lab:dev Nov 26, 2019
antonia-rose pushed a commit to quelltexterin/nemo-uikit-workshop that referenced this pull request Apr 12, 2023
…links

Feature: Add The Ability To Hide Viewall Links
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants