[Core, Docs] add additional props to Hr component

[khushal87]

This PR introduces new props to the Hr component. The props are:

• subHeader
• subHeaderProps
• orientation

Have I added the tests/tested my changes?
Yes

Have I updated the docs for the changes?
Yes

Reference issue: #28

[rtivital]

About inset – currently it feels very complicated, I think it's not necessary as margin can achieve this in single prop + inset is fixed, it is impossible to connect it to layout:

<Hr style={{ marginLeft: 70 }} />
// same as 
<Hr inset={true} insetType="left" />

[rtivital]

First demo does not show dashed variant

[khushal87]

About inset – currently it feels very complicated, I think it's not necessary as margin can achieve this in single-prop + inset is fixed, it is impossible to connect it to layout:

<Hr style={{ marginLeft: 70 }} />
// same as 
<Hr inset={true} insetType="left" />

So adding an insetLength can work? Or do you want the removal of the inset prop? My purpose of having an inset is that it is very common to the Dividers today, and people don't generally feel adding additional styles unnecessarily to it

[khushal87]

First demo does not show dashed variant

Fixed. Thanks for reporting.

[rtivital]

So adding an insetLength can work? Or do you want the removal of the inset prop? My purpose of having an inset is that it is very common to the Dividers today, and people don't generally feel adding additional styles unnecessarily to it

No, I think, insetLength will bring even more complexity – it'll be 3 props to set single margin, does not feel right.

[khushal87]

So adding an insetLength can work? Or do you want the removal of the inset prop? My purpose of having an inset is that it is very common to the Dividers today, and people don't generally feel adding additional styles unnecessarily to it

No, I think, insetLength will bring even more complexity – it'll be 3 props to set single margin, does not feel right.

Okay, will remove it.

[rtivital]

subHeaderProps will be Record<string, any>

[khushal87]

subHeaderProps will be Record<string, any>

Wouldn't it be simply TextProps?
subHeaderProps?: TextProps;

[rtivital]

Wouldn't it be simply TextProps?
subHeaderProps?: TextProps;

This will break component props table

[khushal87]

Wouldn't it be simply TextProps?
subHeaderProps?: TextProps;

This will break component props table

Even LoadingOverlay uses LoaderProps. It works fine according to me.

[rtivital]

Oh, okay, looks good, I was having lots of issues with prop types generator, I think I accidentally fixed them sometime 😄

[khushal87]

@rtivital I was going through the code and then I realized if we could export the StyleProps too from the styles.tsx of every components this will improve the reusability of all the components styles. Suppose I need subheaderProps styles to be of type TextStyleProps this would get easier. What is your opinion on it?
We can do it individually or by making a file that firstly imports this style from every styles.tsx of component and then export it.

[rtivital]

I prefer to keep all props description in open place – component props interface – this way you don't need to jump between files to edit them

[khushal87]

I prefer to keep all props description in open place – component props interface – this way you don't need to jump between files to edit them

What I mean is if we can export TextStylesProps someway and then import it in other style files this would add flexibility. For my usecase if I want to apply color here in subHeader then I have to assign the type of subHeaderProps in Hr.styles.tsx to TextStylesProps which seems impossible now. Did you understand what I mean?

[rtivital]

Yep, I've tried to do so, unfortunately there is no good solution to this. TextStylesProps does not include all Text component props and has required theme field

[khushal87]

Yep, I've tried to do so, unfortunately there is no good solution to this. TextStylesProps does not include all Text component props and has required theme field

Oh, yes it doesn't have all, need to think some other way around.

[rtivital]

Record<string, any> is still the best option

[khushal87]

Record<string, any> is still the best option

Yes, for the style, let me see if this works.

[rtivital]

Great work @khushal87, please fix these issue

[khushal87]

@rtivital Done.

[rtivital]

Thanks! Great work!