patternfly / patternfly

This repo contains core (HTML/CSS) implementation for PatternFly. Issues related to CSS/HTML and layout should be filed here.
https://patternfly.org
MIT License
704 stars 96 forks source link

Evaluate CSS changes necessary to support Web Components #2914

Closed dgutride closed 4 years ago

dgutride commented 4 years ago

Related to https://github.com/patternfly/patternfly-webcomponents/issues/1 and https://github.com/patternfly/patternfly-webcomponents/issues/15

The goal would be to create a POC or use the patternfly-webcomponents repo as a base to understand what is necessary in order to bring in PatternFly component CSS in a manner similar to the way the React components do. Because Web Components typically restrict access to the DOM outside of the component, our current CSS architecture may need to be adjusted in order to support this.

A mixin model has been proposed, is this the ideal solution? Also, what is the implied cost to the team if there are complexity changes in order to move in this direction? How difficult would contributing to PatternFly css become in the future.

Finally - if we do move to a mixin model - what would a migration strategy entail - is this a cookbook where others can convert the current code to build this way? Can we add tests to ensure the output doesn't change for the current PatternFly CSS? Can we avoid another breaking change release to accomplish this by not changing the currently distributed CSS in any material way?

related issue - https://github.com/patternfly/patternfly-webcomponents/issues/15

starryeyez024 commented 4 years ago

I want to elaborate a little on this idea and note that the first step is getting our core variables to match, and then we can build on that to include mixins and placeholders that can be shared by both projects.

The dependencies for these mixins to work in PFE are:

Even if we never got to the point where we had a plethora of shared mixins, it would still be a big win to have shared global variables. :)

WIP exercise on naming is happening here: https://docs.google.com/spreadsheets/d/1Lo567BQlg5132liUNQdFNGPZ0JqPKfYGoylKz3OAzW4/edit?usp=sharing

starryeyez024 commented 4 years ago

One other (perhaps obvious) note about the beauty of mixins, is that you can always add arguments to make them more robust. One way that we've ensured ease of mixin updates in our pattern project, WebRH, is to name arguments when using mixins. That way you could rearrange arguments if needed, and it makes them easier to read when scanning component code.

example:

    @include themes( 
        $map: rule-border, 
        $props: border-color
    );

    @include pfe-accordion-variables($variant: disclosure);
mcoker commented 4 years ago

Just wanted to provide an update on this effort and mention that I’m familiarizing myself with web component technology, the tools available to style them, and reading about best practices for styling web components using global CSS stylesheets.

It seems the best solution is some level of breaking the global CSS into pieces that apply to each component, then using one of various approaches to import the pieces into the web components they style. I think the sass @mixin approach that was recommended would be the best way to do it, and it comes with the added benefit of having mixins available for us to use, making code sharing easier and being able to pass use case specific context to the mixin to manipulate the output.

Next I would like to apply the mixin approach to the page masthead web components in our POC sandbox at https://github.com/patternfly/patternfly-webcomponents and evaluate the effort involved to implement, get an idea if what that might look like applied to the entire library, the impact it might have to our development and review process, and complexity it might add to our existing approach and architecture of PF core CSS.

mcoker commented 4 years ago

Over the last couple of weeks, I've attempted to style the page web component POC, which takes structural elements of the patternfly page component's masthead, and converts them to web components. My focus has been trying to build mixins from the existing patternfly core CSS that can serve to be included in the web component as well as be used to re-construct the existing core stylesheet.

On the whole, the general concept works as expected. There were some problems I ran into, which I'll mention later.

The impact to the overall core stylesheet is that a component styles are no longer written in typical stylesheet form, but broken into pieces (mixins), and the pieces are used as includes to reconstruct a component's stylesheet. Something like:

.pf-c-component {
    // component styles

    &.pf-m-modifier {
        // component modifier styles
    }
}

.pf-c-component__element {
    // component element styles
}

changes to

@mixin pf-c-component {
  // component styles
}

@mixin pf-c-component-modifier {
  // component modifier styles
}

@mixin pf-c-component-element {
  // component element styles
}

.pf-c-component {
    @include pf-c-component;

    &.pf-m-modifier {
        @include pf-c-component-modifier;
    }
}

.pf-c-component__element {
    @include pf-c-component-element;
}

Re-writing patternfly core CSS like this from a basic perspective is a change conceptually, but probably one that we could adopt. This architecture also makes the component styles modular by default, and can help where that may be beneficial - for instance, sharing code between components.

One problem this POC intended to solve was ensuring that parent -> child CSS properties, such as flex & grid, will retain their relationship. This seemed to work fine, with slot { display: contents; }. The one exception I ran into is mentioned below, but I was unable to style children of web components that weren't other web components that I could style with :host in the child web component. If the child was <div class="foo">, styling .foo {} in the web component wouldn't work. This impacts the flex/grid parent -> child relationship, where the child applies styles that work with the parent flex/grid layout - styles such as align-self, order, grid-column/row, etc.

There were some problems I encountered, and I'm not sure if this is a limitation of working with the shadow DOM or a problem I could solve if I continued working on it. But general rules that target children of the web component didn't work. Maybe more succinctly, anything outside of :host {} didn't work. If a web component had a bunch of <div>s as children ( is a good example), placing a wildcard selector like * { border: 1px solid red !important; } in the stylesheet either beneath or inside the :host{} selector did nothing. From what I've read, that should not be the case, yet I couldn't get it to work in this POC. This has pretty far reaching implications if it doesn't work.

From here, I would like to figure out the problem mentioned above. Also, I would like to be able to test other things like .pf-c-component > * > * { display: flex; }, or something to that effect. There are a lot of instances where we redefine component variables inside of completely separate component selectors, often nested in child selectors. And it seems like the POC we built stopped building web components in the ( component, as I would expect the children to be web components, and they're normal HTML elements. Building a more complex POC using nested components, and ones that rely on nested styles and variable overrides in nested styles would shed light on if those types of problems are easily achievable without having to refactor and change the way we write CSS.

mcoker commented 4 years ago

Closing this as the initial investigation into how this might look is complete. We'll open follow-up issues as needed should we choose to further support the use of PF core CSS in web components.