Mix old and new AeIcon

[davidyuk]

I think we should drop size prop because changing of font-size from styles is more flexible.

For example,

<ae-icon size="rem(20px)" />

won't work compared with

.ae-icon {
  font-size: rem(20px);
}

Or you can extract the same size from sevaral icons:

<template>
  <ae-icon />
  <ae-icon />
  <ae-icon />
</template>
<style>
.ae-icon {
  font-size: 20px;
}
</style>

instead of

<template>
  <ae-icon size="20px" />
  <ae-icon size="20px" />
  <ae-icon size="20px" />
</template>

In general, it is a good practice to store markup and styles separately.

Using of <i> for icons breaks HTML semantic: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/i I can't find here anything about the usage of <i> for icons. So, I propose to restore previous behavior (to use <span> instead).

[davidyuk]

font-size change through styles applies in a global scope

Specific styles scope can be set using CSS selectors and scoped attribute of <style>.

Let say we have 2-3 different instances of icons with different sizes, how would you approach that?

I think we should use more specific selectors and/or additional classes.

<template>
  <div class="component-name">
    <header>
      <ae-icon />
      <ae-icon />
    </header>
    <ae-icon class="big" />
  </div>
</template>

<style lang="scss" scoped>
.component-name {
  header {
    .ae-icon {
      font-size: 25px;
    }
  }

  .ae-icon.big {
    font-size: 20px;
  }
}
</style>

[sadiqevani]

@davidyuk

That makes the SCSS too dirty, I don't think having all those classes and modifications on scope level components are good! Lets keep styles as simple as possible.

For now lets keep everything you've added except the change on the size property.

[davidyuk]

That makes the SCSS too dirty, I don't think having all those classes and modifications on scope level components are good!

The last example just uses selectors and nesting, I think we should use SCSS features to reduce code duplication.

Lets keep styles as simple as possible.

Can you implement the last example in a simpler way, without duplication of styles and selectors?

[sadiqevani]

@davidyuk

<template>
  <div class="component-name">
    <header>
      <ae-icon />
      <ae-icon />
    </header>
    <ae-icon class="big" />
  </div>
</template>

<style lang="scss" scoped>
.component-name {
  header {
    .ae-icon {
      font-size: 25px;
    }
  }

  .ae-icon.big {
    font-size: 20px;
  }
}
</style>

This whole thing screams useless applications of in-level css classes.

<template>
  <div class="component-name">
    <header>
      <ae-icon size="25px" />
      <ae-icon />
    </header>
    <ae-icon size="20px" />
  </div>
</template>

Whats all, no need to overwrite CSS.

[davidyuk]

<template>
  <div class="component-name">
    <header>
      <ae-icon size="25px" />
      <ae-icon />
    </header>
    <ae-icon size="20px" />
  </div>
</template>

this is wrong, you have missed styles for the second AeIcon inside the header.

What if there are 10 icons?

<template>
  <div class="component-name">
    <header>
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
      <ae-icon size="25px" />
    </header>
    <ae-icon size="20px" />
  </div>
</template>

is it really better? how I can use rem function in size property?

[sadiqevani]

this is wrong, you have missed styles for the second AeIcon inside the header.

I didn't miss it, I left it on purpose. The font has already a size of 16px, and in case you need to change the font to a bigger one then you can use that property.

is it really better? how I can use rem function in size property?

Think of it this case:

• You can always update the property / size of the icon with a CSS class
• But you can not change the size directly with a property in the component if you remove it.

1. You can always add an extra class
2. Having the property there does not create any problem
3. The person developing can chose his way of modifying the styles, be that either with CSS or modifying directly the component property.

[sadiqevani]

how I can use rem function in size property?

<ae-icon size="1rem" />

[davidyuk]

I didn't miss it, I left it on purpose. The font has already a size of 16px, and in case you need to change the font to a bigger one then you can use that property.

Sorry, I don't understand.

But you can not change the size directly with a property in the component if you remove it.

Actually, I can:

<ae-icon style="font-size: 25px" />

Having the property there does not create any problem

It is a bad precedent, in future you may want to create a lot of properties like this one in other components that will take time to implement and maintain instead of using already implemented well-known way to mutate styles of Vue components. Keep it simple.

<ae-icon size="1rem" />

It is 'natural' rem, I want to be able to use SCSS function to convert pixels value to rems.