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

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

Open
lynnmercier opened this issue Mar 15, 2018 · 2 comments
Labels

Comments

@lynnmercier
Copy link
Contributor

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
Copy link
Contributor

kfranqueiro commented Mar 16, 2018

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:

  • We wouldn't want this behavior when we're using it in the context of purely .mdc-bar (this is probably detectable at least in the simplest case of the selector only being that)
  • It doesn't detect its use within compound selectors e.g. .my-modifier.mdc-bar which IIUC ideally would also remove just the .mdc-bar. I don't think this is even possible to do within Sass without risking false positives (e.g. looking for .mdc-bar but ending up matching .mdc-bark and replacing all but the last letter).

@kfranqueiro
Copy link
Contributor

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants