search

inovex / elements

Lovingly crafted ui components based on web components. Works well with all Frameworks - including Angular, React and Vue.
https://elements.inovex.de
MIT License
69 stars 9 forks source link

ino-list: Use sass mixins provided by MDC #241

Closed janivo closed 3 years ago

janivo commented 3 years ago

The new versions of the MDC list provide a whole lot of mixins we can use in order to cleanup our code. See: https://github.com/material-components/material-components-web/tree/v8.0.0/packages/mdc-list#sass-mixins

P1NHE4D commented 3 years ago

I took a brief look at the ino-list, ino-list-item, and ino-list-divider stylesheets and they are actually not as packed as it is the case with some other components. What is more, there aren't any mixins that could be used to replace the state-dependent styles (hover, active, focus), which account for the majority of the lines in the ino-list-item stylesheet. Maybe I am overlooking something, but I don't think it's necessary to replace the existing styles with the mdc mixins.

janivo commented 3 years ago

For example, couldn't we replace this: https://github.com/inovex/elements/blob/8b91f4e51abf5a920ccf3718ccc54e13802768f1/packages/elements/src/components/ino-list-item/ino-list-item.scss#L69-L76

With something like this:

.mdc-list-item { 
   @include item-primary-text-ink-color(var(--list-item-deselected-color))
   @include item-secondary-text-ink-color(var(--list-item-deselected-color))

   background-color: var(--list-item-deselected-background-color); 
// ...
silentHoo commented 3 years ago

As we're not see any benefit from saving two lines of code, we close the issue.