Add custom HTML props

[Seth10001]

Implements the #353 feature.

At first I started by extending CommonProps with React.HTMLAttributes<HTMLSpanElement> so that you could pass arial-label down to the top-level span like <Loader arial-label="loader" />. However, it's tricky to pull out only the HTML props with there being different prop interfaces for some components.

Additionally, commonValues was producing an error because it's required to define all the props, which in this case was 200 HTML props: https://github.com/davidhu2000/react-spinners/blob/5e23db437cc31170a646103a3fe81b9ae5e2a080/src/helpers/proptypes.ts#L12

Given these issues, I decided to put it in it's own prop customProps. What do you think about that? I'm not entirely convinced the name is the perfect fit.

The tests are failing because of the #455 error.

[davidhu2000]

thanks for the contribution. looks like, in terms of usage, we have two options

1. your current implementation

   <Barloader
   loading={true}
   ...
   customProps={{ 'aria-label': 'label' }}
   />

2. allow any prop that is available and pass that down

   <Barloader
   loading={true}
   ...
   aria-label='label'
   />

which is what I believe you mention you had an issue with. However, I think 2 is probably a better end-user experience.

To address the issues you had

However, it's tricky to pull out only the HTML props with there being different prop interfaces for some components. I think extending the common props makes sense here.

Additionally, commonValues was producing an error because it's required to define all the props, which in this case was 200 HTML props:

Does something like this work?

const commonValues: Required<CommonProps> & Partial<React.HTMLAttributes<HTMLSpanElement>>

[Seth10001]

Yeah I agree, option 2 is the better user experience! Your solution fixes part of the issue I had, I was just having some issues pulling out only the HTML props in each render functions. I can change this PR to what I had originally had to illustrate the issues better

[Seth10001]

I made an example of what it could look like for SquareLoader.tsx https://github.com/davidhu2000/react-spinners/blob/0e009d586b8b388a84daa6772501e856a8b9d6f7/src/SquareLoader.tsx#L32-L34

This method is a little clunky because of two things. The first is color, speedMultiplier and size are required to be here so they don't get passed as a prop down into span, but they go unused in the actual function. Secondly if any new props are added we'll have to remember to add them to this list otherwise we risk getting them passed down to span and throwing an error.

Any ideas how to fix this particular issue?

[davidhu2000]

@Seth10001 i have a implementation in #476 that works pretty well. it is part of the function component conversion which make this less clunky.

[Seth10001]

@davidhu2000 Looks and work great. Good job!

I noticed you created a new props.ts file and defined the props there to I assume get around dealing with Required. Do you plan on removing the interfaces.ts file? https://github.com/davidhu2000/react-spinners/blob/aee6124bcf6a603983d8eafbe334c97ad32d8bd2/src/interfaces.ts#L17-L20

[davidhu2000]

@davidhu2000 Looks and work great. Good job!

I noticed you created a new props.ts file and defined the props there to I assume get around dealing with Required. Do you plan on removing the interfaces.ts file?

https://github.com/davidhu2000/react-spinners/blob/aee6124bcf6a603983d8eafbe334c97ad32d8bd2/src/interfaces.ts#L17-L20

Yes, that's the plan. I'm doing it this way so i can have both old/new live in harmony while i do the migration

[davidhu2000]

this change is implemented. we should be ready to publish another alpha build. 0.13.0-alpha.4