Discussion to refactor icon inclusion template

[chemoish]

In similar spirit to https://github.com/videojs/video.js/issues/1880.

@each $name, $content in $icons {
  .vjs-icon-#{$name} {
    font-family: $icon-font-family;
    font-weight: normal;
    font-style: normal;

    &:before {
      content: char($content);
    }
  }
}

https://github.com/videojs/font/blob/master/scss/_icons.scss#L74

---

The code above specifies the font-family for every icon—making overriding difficult. My recommendation would be to separate that out and have a mixin be created to include the font-family and associated props only when needed.

For controls such as play/pause or volume-0/volume-1/volume-2/volume-3, it really only requires the inclusion of the font-family on the control button. However, due to specificity rules I need to override everyone of them.

/// @name VideoJS Volume

:global(.vjs-volume-menu-button)
{
  :global(.video-js) &
  {
    @include icon-font;

    &::before
    {
      content: $icon-volume-3;
    }

    &:global(.vjs-vol-0)
    {
      @include icon-font;

      &::before
      {
        content: $icon-volume-off;
      }
    }

    &:global(.vjs-vol-1)
    {
      @include icon-font;

      &::before
      {
        content: $icon-volume-1;
      }
    }

    &:global(.vjs-vol-2)
    {
      @include icon-font;

      &::before
      {
        content: $icon-volume-2;
      }
    }
  }
}

---

Definitely not a 1:1 translation, but this is the way I manage custom icons.

/// Icon Font: <%= font_name %>

/// Font-face styles

@mixin icon-font-face {
<%= font_face(path: @font_path_alt) %>
}

/// Font styles

@mixin icon-font {
<%= glyph_properties %>
}

/// Font glyph

@mixin icon-glyph {
<%= glyphs %>
}

/// Font variables

<% @glyphs.each do |name, value| %>
$icon-<%= name.to_s %>: "\<%= value[:codepoint].to_s(16) %>";<% end %>

I then have the flexibility to only include icon-font when necessary. Additionally, I need to add the appropriate content: $icon-{name}.

[gkatsev]

personally, I feel like moving away from sass (and some others have similar sentiments). However, I think what should really happen is that these classes are created and then used directly rather than mixing them into the buttons. So, the play toggle should have a class of vjs-icon-play and that the vjs-icon-play won't be mixed in or extended into the vjs-play-toggle class.

[gkatsev]

Having these live as separate classes that are used should make it easier to override, I'd hope.

[chemoish]

I guess, I still really like sass, so I cannot really comment, but regardless of the processing, if you have each icon bake everything into it, then you will need to repeat code for every icon.

If you had a .vjs-icon class, you can then bind font-family one time and in one place. And that could be overwritten also in one place.

Just my two cents.

Feel free to close if there is already a solid direction you guys are headed in whatever repository.

[gkatsev]

Having a vjs-icon and a vjs-icon-play makes sense to me. That's what font-awesome does as well.

[mmcc]

I had 2 classes in my first stab at this, but at this point I don't remember if I had a good reason for moving to one. I think all of this makes sense, though, I think moving the font-family declaration to a new class is reasonable.

personally, I feel like moving away from sass

I spent a while messing with CSSNext a few months ago and had a disastrous experience. 10/10 would not do again (for a while).

[gkatsev]

@mmcc but we need to move away from sass or at least fix our usage of sass because https://github.com/videojs/video.js/issues/3692. I don't think in videojs we're using anything specific of sass that isn't easily usable in postcss or just plain-old css. But that's a matter for another discussion :)