search

antonreshetov / vue-eva-icons

Is a pack of more than 480 beautiful open source Eva icons as Vue components
https://antonreshetov.github.io/vue-eva-icons/
MIT License
196 stars 5 forks source link

Adding hover property. #5

Closed gedrick closed 5 years ago

gedrick commented 5 years ago

Adds a hover property for changing the icon color on mouse hover.

antonreshetov commented 5 years ago

Hi @gedrick

Thank you for PR, but it is not clear why this decision. If there is such a need, then it can be solved simply by CSS:

<style lang="scss">
svg.eva {
    &:hover {
      fill: #409eff;
    }
  }
</style>

It may also be useful to you in the future.

  1. Not quite clear definition of the property hover since it's just an action, is better to use hover-fill which describes the immediate action.
  2. Data currentColor is not a dynamic property, therefore it is better to use for this a computed property.
  3. Commit does not match the сonvention.
  4. It’s not good to add changes to one commit that are not directly related to it, such as changing documentation.
gedrick commented 5 years ago

That's a good point, I was having trouble figuring out how to get a hover color to change. I suppose that will work fine after all. This can be closed 👍