Skip to content

Conversation

@sghoweri
Copy link
Contributor

@sghoweri sghoweri commented Sep 15, 2018

At a high level, this PR further chips away at the UIKit Refactor work that's going on over the past couple months.

Some of the worthwhile changes worth pointing out include:

  1. Cleaning up and polishing a bunch of UIKit code from the past couple UIKit refactor PR's
  2. Squashing a bunch of cross-browser bugs (especially in IE 11 and Firefox)
  3. Adding in basic cosmic config support to the Webpack build (allowing for more customizable UIKit builds moving forward) and
  4. The addition of a few new Web Component-based components (powered by Preact, JSX, Redux with localstorage caching, and SkateJS) to get some feedback on how I’m imagining PL's UI can further get extended and customized moving forward, while still being as framework-agnostic as possible.

For now, this PR is trying to start off really small by only including 3 new custom elements: <pl-toggle-theme> and <pl-toggle-layout> that handle toggling the layout and theme options in PL, plus another <pl-layout> component that listens for state changes and adds the appropriate classnames automatically.

Also worth mentioning is this PR also takes care of all the upfront build tool, Babel config, and cross-browser polyfilling work required to make sure these changes work across the board (IE 11 included)

Side note: I've already got some juicer refactored components on the back-burner as well (Typeahead Search, refactored Modal Viewer, etc) that are almost all ready to go as well but I want to make sure the initial upfront work is understood so we can tackle this next phase of evolving Pattern Lab’s UI one step at a time. 🙂

pattern_lab_-_styleguide-brand-colors

pattern_lab_-_styleguide-brand-colors

pattern_lab_-_styleguide-brand-colors

pattern_lab_-_styleguide-brand-colors

…nal classnames when rendering, and feature check for Shadow DOM support
… display inline isn't centered as expected for example.
…ithin JavaScript components + add support for JS components loading Sass files that aren't intended to be inlined; add support for externally exposing build config options via cosmic config
… toggling the theme and layout config options
…es by the <pl-toggle-theme> and <pl-toggle-layout> components
…dling the flex box direction changes for the modal / viewport components
…e of the page; update to account for vertical vs horizontal layouts
…load order to account for the styleguide.js logic being sensitive to the order that things load in
@bmuenzenmeyer
Copy link
Member

Hey @sghoweri ! 👋

Thanks so much for this. Apologies for the slower reply. I will take a look ASAP

@sghoweri
Copy link
Contributor Author

sghoweri commented Sep 25, 2018

Hey thanks @bmuenzenmeyer, no worries!

Let me know if there’s anything else I can do to help get this out the door (any changes, questions, etc) — really excited about this one!

We’ve already got a version of the code for this up on staging which, except for the one unrelated Search-related bug I mentioned in #949 (probably something silly I messsd up at the last minute) the updates from both of these PR are working beautifully!

@bmuenzenmeyer
Copy link
Member

@sghoweri Hoping to review this proper soon

@sghoweri
Copy link
Contributor Author

sghoweri commented Oct 15, 2018

@bmuenzenmeyer given the importance of getting an MVP version of v3.0 shipped, I'd be totally OK if you think it makes more sense to review but not necessarily release these UI updates till after we get past the initial 3.0 release.

Personally, I think the community would be PUMPED to see visible progress towards making Pattern Lab's UI more modern and extendable in the initial v3.0 release (even if there's still some ongoing UI refactor work) -- hence why I've been doing my best to try and make these refactor updates as incremental and non-backward breaking as possible.

That said, I also want to be mindful that these updates aren't a surprise bottleneck holding everything back either!

@bmuenzenmeyer
Copy link
Member

@sghoweri

I am having trouble testing this.

I've pulled it down, bootstrapped the monorepo again, and rebuilt the uikit just in case
When I try to run npm run pl:serve within packages/development-edition-engine-handlebars I get the following:

image

@sghoweri
Copy link
Contributor Author

sghoweri commented Oct 23, 2018

@bmuenzenmeyer oh - that error was from a file change in Google’s community-maintained polyfills upstream.

I already went in and fixed this in the other PRs by updating the package.json file + updating one line in the polyfills.js file — I’ll need to go in and do the same for this one.

sghoweri@72c0168#diff-d93f7a4be35cabaf5729f725702a9280

sghoweri@72c0168#diff-e756faf6983689c170147ebe05d614d4

Update NPM scripts to include a uikit-specific build command + consolidate `setup` command
Update Travis commands to bootstrap Lerna + automatically run a fresh uikit build
Add missing style-loader package causing a silent error
Switch on bail flag by default when running a full Webpack build so unexpected errors get caught by Travis
@sghoweri
Copy link
Contributor Author

@bmuenzenmeyer take another look at this when you get a sec!

I found a dependency was missing from uikit on this older branch (which explains why the build was mysteriously failing) so I also fixed that + tweaked the NPM scripts so uikit builds on Travis early on + bails more consistently

@bmuenzenmeyer
Copy link
Member

@sghoweri gave this another go

pulled branch
npm run bootstrap
cd packages/uikit-workshop
npm run build
cd ../development-edition-engine-handlebars
npm run pl:serve

image

@sghoweri
Copy link
Contributor Author

@bmuenzenmeyer besides the handlebars dev edition you tried running (which I’ll troubleshoot tonight and run through those testing steps — it’s crazy I didn’t run into these issues running these UIKit updates in Pattern Lab PHP 2), are there other testing commands / testing steps for other engines I should be running through in PL Node locally to confirm this works everywhere as expected?

@sghoweri
Copy link
Contributor Author

^ this particular issue sounds like it could be unescaped opening / closing HTML tags that handlebars doesn’t know how to deal with

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Oct 26, 2018 via email

@sghoweri
Copy link
Contributor Author

@bmuenzenmeyer figured out the problem.... I put in the wrong public path in the Webpack config. 🤦‍♂️

image

@bmuenzenmeyer
Copy link
Member

@sghoweri

are there other testing commands / testing steps for other engines I should be running through in PL Node locally to confirm this works everywhere as expected?

I am focusing most of my testing to this edition going forward, while we should also test major releases against the starterkit-mustache-demo.

My hope is that we can eventually pivot to a more robust handlebars demo that is both useful in it's own right (my development-edition is sorta the makings of a styletile) and demonstrates handlebars features. When this solidifies, it'd be stellar to cover rendering of those templates all the way to the UI as functional tests in CI and netlify/now

hope this helps

@bmuenzenmeyer
Copy link
Member

it’s crazy I didn’t run into these issues running these UIKit updates in Pattern Lab PHP 2

you likely didnt because of differences in how we package up some of the assets or place them in the finished public folder. I've always aimed for feature parity, but not to an identical extent.

@bmuenzenmeyer
Copy link
Member

@sghoweri

Tested in IE11 and Chrome.
This is amazing work Salem! Thanks for being patient with me!

@bmuenzenmeyer bmuenzenmeyer merged commit e56de43 into pattern-lab:dev Oct 27, 2018
antonia-rose pushed a commit to quelltexterin/nemo-uikit-workshop that referenced this pull request Apr 12, 2023
…or-p5--web-components

UIKit Refactor Part 5: Redux + SkateJS + Preact-Powered Web Components
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