pattern-lab / styleguidekit-assets-default

The static assets for the default StyleguideKit for Pattern Lab. Contains styles and mark-up for Pattern Lab's front-end.
http://patternlab.io/
MIT License
35 stars 67 forks source link

Add config options for themes #91

Closed bradfrost closed 6 years ago

bradfrost commented 6 years ago

Right now we have the following theme options that are ready to ship. We need to add config options for the following options that get appended to PL's pl-c-body:

Here's what these options could look like inside of patternlab-config.json, with the default (aka current) values displayed first:

{
  "theme" : {
    "color" : "dark" | "light",
    "density" : "compact" | "cozy" | "comfortable",
    "layout" : "horizontal" | "vertical"
  }
}

I know which classes need applied, but I don't know how to wire up the config and write the conditionals inside of src/html/partials/index.html. Could use some help, @bmuenzenmeyer or @EvanLovely.

bmuenzenmeyer commented 6 years ago

hmm, yeah since that isnt a template at all (rather, just plain HTML), we may need to pursue a different approach or refactor this a bit

will think on it - i know how we could do it in short order, as long as you are okay with a potentially momentary flash of the default ui ?

bmuenzenmeyer commented 6 years ago

candidates for this code would be styleguide.js or layout.js. both should have access to the provided config

bradfrost commented 6 years ago

candidates for this code would be styleguide.js or layout.js. both should have access to the provided config

Are you saying the code should live in one of those two files? I didn't want to tackle a giant refactoring of JS, but like the SCSS file structure, the JS could afford to be chunked out into more modular chunks. I'd be curious to hear if you also think that's a good idea, but personally I would make a new themes.js file or something that handles the functions. styleguides.js should be about 5 different files :)

bmuenzenmeyer commented 6 years ago

I have no shortterm desire to further refactor the frontend javascript or template engines. I think it amounts bikeshedding, and may only be necessary when we look into the JSON schema feature, if at all.

I am saying that the easiest path forward, avoiding major refactor (since the index.html is not a template that gets compiled with data), would be to have the existing javascript code read from the exposed theme config and apply classes to the PL body as needed. I was commenting mostly for my own purposes.

something like ... (enter shitty pseudocode)


//define map of key to actual css class
var themeColorValues = {
 'light': 'pl-c-body--theme-light'
}

//assign value based on mapping (if found)
var colorClass = '';
if (config && config.theme && config.theme.color) {
   colorClass = themeColorValues[config.theme.color];
}

//apply class to body
document.querySelector('.pl-js-iframe').classList.add(colorClass);
bmuenzenmeyer commented 6 years ago

the JS could afford to be chunked out into more modular chunks. I'd be curious to hear if you also think that's a good idea, but personally I would make a new themes.js file or something that handles the functions. styleguides.js should be about 5 different files

I should read that more closely too. sorry. we can definitely place the new logic in a new file. the build process should concatenate it all together anyways. I think the larger thing to figure out is whether it will be fast enough to avoid an ugly UX. If there is a problem, we will to get more creative.

bradfrost commented 6 years ago

I should read that more closely too. sorry. we can definitely place the new logic in a new file. the build process should concatenate it all together anyways

Sounds good. We're on the same page.

I think the larger thing to figure out is whether it will be fast enough to avoid an ugly UX. If there is a problem, we will to get more creative.

Yeah, I guess my server-side rendering brain immediately went to Mustache hooks for the theme stuff to avoid flashes of default theme before config options get loaded in. Of course, I have no idea what the level of effort is to achieve that. I will say this is relevant to other not-too-distant things like user-defined PL CSS, user-defined logos, and other things. Do we want that stuff to be baked into the HTML, or added with JS?

bmuenzenmeyer commented 6 years ago

@bradfrost check out that branch. not perfect. but something!

EvanLovely commented 6 years ago

OK, config.theme is exposed to use on the PHP side.

EvanLovely commented 6 years ago

I've also got a PR up for implementing this: #93

@bmuenzenmeyer Does yours fire before render or after? As in: do you see the UI move around?

EvanLovely commented 6 years ago

@bradfrost Re:

Do we want that stuff to be baked into the HTML, or added with JS?

It's gotta be JS as the config is in JS and that HTML isn't a template that gets passed data from config.

bmuenzenmeyer commented 6 years ago

@EvanLovely let's go with your PR. My js file solution caused a flash, at least in my quick test.

bmuenzenmeyer commented 6 years ago

In the future we should try to coordinate better too :)

EvanLovely commented 6 years ago

👍🏼 On both comments

bmuenzenmeyer commented 6 years ago

closed via https://github.com/pattern-lab/styleguidekit-assets-default/releases/tag/v4.0.0-alpha.2

awesome work @bradfrost and @EvanLovely