JoshK2/vue-spinners-css

Help wanted! - support size property

Closed this issue ยท 5 comments

Hey, I want to add support to size property like I started to do in the react project
https://github.com/joshK2/react-spinners-css#-list-of-spinners---proptypes-and-default-props

If someone wants to help me, it's with pleasure.

Can I work on this?

Hey @guesant, with pleasure!
I just commit and update to Circle Loader to support size property, you can take a look at the code https://github.com/JoshK2/vue-spinners-css/blob/master/src/components/CircleLoader.vue

I think it's a good idea to do this upgrade to the Ring Loader because it's an easy win.
Let me know if you have any problems.

Hey @guesant, with pleasure!

Thanks! ๐Ÿ˜

I just commit and update to Circle Loader to support size property, you can take a look at the code https://github.com/JoshK2/vue-spinners-css/blob/master/src/components/CircleLoader.vue

I had come to this same implementation of Circle Loader โœŒ๏ธ

Let me know if you have any problems.

I've two questions:

  1. The size property will be interpreted as px. What do you think in the future support the other units?
    Later I can send a PR that adds the suport of using other units (px, in,% etc).

  2. I have seen that in the code there are several repeating tags that can be simplified by the v-for directive.

Like you do here:

const circles = [...Array(12)].map((_, index) => <div key={index} style={{ background: `${color}` }} />)

What do you think about this?

  1. The size property will be interpreted as px. What do you think in the future support the other units?
    Later I can send a PR that adds the suport of using other units (px, in,% etc).

I think is a good idea to support also percentage unit, but first, we should support basic pixel size.

2. I have seen that in the code there are several repeating tags that can be simplified by the v-for directive.

This improvement is welcome! ๐Ÿ˜ƒ