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

Re-architect frontend UI code #72

Open
bradfrost opened this issue Sep 9, 2017 · 25 comments
Open

Re-architect frontend UI code #72

bradfrost opened this issue Sep 9, 2017 · 25 comments
Assignees
Milestone

Comments

@bradfrost
Copy link
Member

In an effort to make Pattern Lab's UI more flexible and extensible, the frontend code should be re-architected. I'll likely be tackling this using these conventions, which have proven to be effective on very large, complex software applications. So it should work well here.

Given the nature of the changes, we'll have to be mindful of what would be affected (plugins, dependencies, etc), so could use some guidance in flagging that stuff.

@bradfrost bradfrost added this to the vNext milestone Sep 9, 2017
@bradfrost bradfrost self-assigned this Sep 9, 2017
@bradfrost
Copy link
Member Author

Alright I'm going to give a blow-by-blow here. Working on the refactor in this branch: https://github.com/pattern-lab/styleguidekit-assets-default/tree/feature-css-refactor

@bradfrost
Copy link
Member Author

bradfrost commented Sep 9, 2017

The existing styleguide.scss contains all of the styles, which is unruly. I've chunked things out into the following folders within an scss folder:

  • abstracts for variables
  • base for reset
  • components for UI components
  • utilities for helper classes
  • vendor for third-party CSS libraries

@bradfrost
Copy link
Member Author

bradfrost commented Sep 9, 2017

@bmuenzenmeyer @EvanLovely just a heads up that globally I'm going to restructure the current prefix of sg- (standing for "style guide") to pl- (standing for "Pattern Lab" of course). That SG was in place before Pattern Lab had a name, and I think the pl- prefix is more helpful as an identifier.

I bring it up as a heads up because that obviously touches a lot of things.

@bradfrost
Copy link
Member Author

I cleaned up the Sass variables and updated them to be more descriptive. So instead of $sg-gray-light-3 we're now working with $pl-color-gray-20.

@bradfrost
Copy link
Member Author

I renamed the main CSS file to pattern-lab.css and removed minification of the file.

@bradfrost
Copy link
Member Author

I've been making a whole mess of refactoring tweaks to the header. A lot of it is syntax and cleanup, but I'm also starting to work on theming. Here's a taste of what a light theme and a "comfy" (read: bigger padding) theme could look like:

theme-wip

This ultimately can of course lead to a left sidebar version vs an across-the-top header, similar to http://bradfrost.github.io/style-guide-guide/

And this also paves the way for users to more easily theme their own Pattern Labs.

@bradfrost
Copy link
Member Author

The frontend refactoring bleeds into https://github.com/pattern-lab/styleguidekit-mustache-default, so I've been going through and updating that code as well.

@bradfrost
Copy link
Member Author

I also want to change the styleguide directory that ultimately lives in public to pattern-lab, and put everything that belongs to PL into that directory. That includes annotations and anything else that can be put in there.

@bmuenzenmeyer
Copy link
Member

@bradfrost

I also want to change the styleguide directory that ultimately lives in public to pattern-lab

let us know if that lands into a branch or branches, as it will likely break PL Node and PL PHP

@bradfrost
Copy link
Member Author

@bmuenzenmeyer Yeah I realize that's going to be a big change, so I'll make it it's own thing and flag/coordinate things accordingly.

@sghoweri
Copy link

let us know if that lands into a branch or branches, as it will likely break PL Node and PL PHP

@bmuenzenmeyer that's precisely why I had originally suggested (and had already gotten a good deal into) doing this Styleguidekit Assets Default re-architecture as a monorepo using something like Lerna -- at least for the initial buildout -- given how even a relatively small change in one Pattern Lab dependency can require changes spanning 2 or 3 other repos...

@bmuenzenmeyer
Copy link
Member

I've been kicking around the idea of putting node core together with all node engines, but I don't see a lot of logic to putting patternlab node and patternlab php together.

@bradfrost
Copy link
Member Author

I think generally speaking if there's a way to move all UI into one one repository (styleguidekit-assets-default) that makes sense. I can't speak to the engine stuff, but if there's a way to get the UI-specific stuff that lives in styleguidekit-mustache-default etc into styleguidekit-assets-default that would be good I think.

@sghoweri
Copy link

@bmuenzenmeyer sure, no arguments there.

But then again, what if we only really needed one single Pattern Lab core to rule them all to handle any underlying business logic and that just hooked in with whatever template adapter we needed (PHP-powered Twig, React, Handlebars, vanilla web components, etc)?

Even just yesterday I stumbled upon a brand new Drupal 8 module that has PHP talking to a sibling Node server to handle server-side rendered web components (on top of the node-twig project which lets Node render templates using the native PHP Twig Engine) so I've just been doing a lot of thinking about how Pattern Lab as a whole could become even more universal (and not require us to do a huge rebuild every couple of years)

@bmuenzenmeyer
Copy link
Member

But then again, what if we only really needed one single Pattern Lab core to rule them all to handle any underlying business logic and that just hooked in with whatever template adapter we needed (PHP-powered Twig, React, Handlebars, vanilla web components, etc)?

You bet! I humbly think that Pattern Lab Node is there.

@bradfrost
Copy link
Member Author

@sghoweri

what if we only really needed one single Pattern Lab core to rule them all to handle any underlying business logic and that just hooked in with whatever template adapter we needed (PHP-powered Twig, React, Handlebars, vanilla web components, etc)?

I think to @bmuenzenmeyer's point that's the goal!

and not require us to do a huge rebuild every couple of years

I don't think the big efforts every couple years is due to keeping up with the different engines, but really evolving the whole platform to be more robust, versatile, and manageable. In any case, I think we're all on the same page of trying to put the right architecture in place to make that happen!

@bradfrost
Copy link
Member Author

Alright, big update! I think I have most everything done regarding the refactor of the UI code. All CSS is rearchitected, all JS hooks are standardized, all of the features are present and accounted for.

The one thing that's holding me back is that the single pattern view appears to choke.

screen shot 2017-10-05 at 1 27 52 pm

I can continue hunting this down, but as of right now that's my only blocker. Once that's resolved, I can test the annotations feature and then we should have a brand new, shiny, slimmed-down, scalable, extensible, themeable Pattern Lab UI!

If either @bmuenzenmeyer or @EvanLovely wants to help get to the bottom of that issue, that would be freaking great.

@bmuenzenmeyer
Copy link
Member

@bradfrost i glossed through your commits a day or so ago but nothing stood out - did you rename or move any files as part of this?

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Oct 5, 2017

useful debugging inside src/html/index.html

	<!-- load the Pattern Lab viewer js -->
-	<script src="styleguide/js/patternlab-viewer.min.js"></script>
+	<script src="styleguide/js/patternlab-viewer.js"></script>

then upon rebuild (assuming you are linked) you can at least see the code

this is what I get outta the gate

image

will have more time to look into this later

** Edit ** For this specific error, I had some crap cookies saved. Debug steps still useful.

@bmuenzenmeyer
Copy link
Member

The suffixes are not being added to the requested file

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Oct 5, 2017

It looks like 5d06bf7 is not in your codebase.

Getting these changes back in will do the trick!

@bradfrost You must not have started with master ?

Makes me worried that other changes might not be reflected: https://github.com/pattern-lab/styleguidekit-assets-default/commits/master

@bmuenzenmeyer
Copy link
Member

bmuenzenmeyer commented Oct 5, 2017

@bradfrost pull changes, re-build, and try again :)

Looks like something similar needs to happen for the code panels. I can look into that too, unless you can figure it out. Lots of errors.

Not bad for a bus ride home! 🚌 ⚡ ⌨️

@bradfrost
Copy link
Member Author

Alrighty, update on my end. Everything has been refactored and is looking/working properly in Chrome, Firefox, and Safari on my Mac. I don't have a Windows machine on me (I'm on the road) and haven't had a chance to Browserstack it, so Windows testing still needs to be done.

Assuming that things check out on that front, we're good to do whatever needs to be done to get these changes where they need to go. I'll leave that to @bmuenzenmeyer.

@bradfrost
Copy link
Member Author

Alright, I've gone through everything on Browserstack and after some refactoring of the tabs things are looking and functioning great in Edge and IE11. So we should be good to go!

@bmuenzenmeyer, I'd like to continue work on some of the theming/enhancement stuff in other branches. What would recommend as far as branching goes? Should I start a new branch from this? Wait for you to do your thing then branch from there? Something else?

@bradfrost bradfrost mentioned this issue Oct 8, 2017
@bmuenzenmeyer
Copy link
Member

@bradfrost you can branch from here and I can keep it all in sync.

@bradfrost bradfrost mentioned this issue Oct 9, 2017
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants