material-components / material-components-web

Modular and customizable Material Design UI components for the web
https://material.io/develop/web
MIT License
17.15k stars 2.14k forks source link

Figure out how to call Sass mixins from other components, within the baseline of a parent component #2415

Open lynnmercier opened 6 years ago

lynnmercier commented 6 years ago

Take this made up example of the mdc-foo component using the mdc-bar component in its internal implementation:

<div class="mdc-foo">
  <div class="mdc-bar"></div>
</div>

And there exists some Sass mixin to customize the color of mdc-bar

@mixin mdc-bar-color($color) {
  color: $color;
}

Now lets say that the mdc-foo component wants to have a blue mdc-bar in the default state, but a red mdc-bar in the invalid state. Then the stylesheet for mdc-foo has

.mdc-foo {
  .mdc-bar {
    @include mdc-bar-color(blue);
  }
}
.mdc-foo--invalid {
  .mdc-bar {
    @include mdc-bar-color(red);
  }
}

which outputs this CSS, once Sass is compiled.

.mdc-foo  .mdc-bar {
  color: blue;
}
.mdc-foo--invalid .mdc-bar {
  color: red;
}

...so far so good...

The problem is if we change the implementation of mdc-bar so that it has a sub-element

<div class="mdc-bar">
  <div class="mdc-bar__sub"></div>
</div>

And we have to update the implementation of the Sass mixin mdc-bar-color

@mixin mdc-bar-color($color) {
  .mdc-bar__sub {
    color: $color;
  }
}

All these changes happen in the mdc-bar node module, and none of them are breaking changes...but now the mdc-foo stylesheet outputs

.mdc-foo  .mdc-bar .mdc-bar__sub {
  color: blue;
}
.mdc-foo--invalid .mdc-bar .mdc-bar__sub {
  color: red;
}

which is unnecessarily specific

How do we handle this case more gracefully?

kfranqueiro commented 6 years ago

I experimented with this because I was curious, but I definitely don't have a silver bullet.

https://gist.github.com/kfranqueiro/6a47cafa274f2d774b2d133ed7750f02

(Also disclaimer it could use a bit of cleanup, probably)

This tries to pretty exhaustively replace .mdc-bar with .mdc-bar__sub otherwise append it to the selector, but I can already identify a couple of problems:

kfranqueiro commented 6 years ago

It also occurs to me that maybe this is something we should seek to solve via documentation rather than implementing safeguards to attempt to tweak parent selectors, which seems likely to be error-prone.

I've already wondered this after seeing how some of our components' mixins include a selector vs. how some don't (e.g. look at how mdc-toolbar-icon-ink-color works vs. mdc-tab-icon-ink-color)...

Do you think it's viable to include some indication in our mixin documentation tables as to whether the mixin includes a sub-element selector, and write some centralized guidance somewhere on what that means for using it within custom selectors?