Blazorise: Property unifications

Created on 12 Jul 2020  ยท  13Comments  ยท  Source: stsrki/Blazorise

900, #1087

Currently input components like TextEdit, NumericEdit, Select, Check etc. are using Size enum to define the size.

public enum Size
{
    None,
    ExtraSmall,
    Small,
    Medium,
    Large,
    ExtraLarge,
}

But the problem is that most of the frameworks only support Small, Medium and Large implementations on input element. Also some components like Check and Radio doesn't support any sizing and since they are inherited from the same BaseInput class they also have the Size property.

When I started Blazorise I used Size for everything and obviously now is the time to clean the API.

There are few ways to proceed.

  1. Introduce new enum named InputSize that will have only the most supported values, Small and Large. This is the easiest way as most of the API will stay the same.
  2. For every component group add new size enum, like InputSize(for TextEdit, DateEdit, etc.), CheckSize, SelectSize, etc.
  3. Leave Size as it is and then implement missing sizes in CSS and theme generator. This one requires a lot more work but I'm not sure if it's the right time to do it.

So for now I think best is to go with first choice. Clean the API, unify all sizes across the components and then in one of the next milestone(0.9.4 or 0.9.5) add more options like ExtraSmall and ExtraLarge.

@MitchellNZ and @WillianGruber I would like your opinion before I start on this. tnx

Discussion โ˜• Refactoring ๐Ÿงน

All 13 comments

This sounds like a good plan to me @stsrki
I really like the idea of unifying attributes where it makes sense, #1087 is a good example!

Also, I'm happy to help with this where I can, if you need it

Yeah I think it's best to just make it simpler now. I started doing sizing of different components but it would just take too long. So it's better to just clear the API instead. And the entire plan for 0.9.x to 1.0 milestone is exactly that, to clear the API so onxe we reach the 1.0 we can say that it's nice and stable.

One thing I cannot decide on which is better. For example for typography components like Text, Paragraph, Heading etc. Is it better to have properties:

1. Color, Alignment, Transform, Weight

<Paragraph Color="TextColor.Primary" Alignment="TextAlignment.Right"></Paragraph>

OR

2. TextColor, TextAlignment, TextTransform, TextWeight

<Paragraph TextColor="TextColor.Primary" TextAlignment="TextAlignment.Right"></Paragraph>

Introduce new enum named InputSize that will have only the most supported values, Small and Large. This is the easiest way as most of the API will stay the same.

Sounds good to me.
About naming, I'm always inclined to "more descriptive, the better", as it's a kind of self-documentation, but that's a personal taste.

So basically as long as it's same across components, no mater the style, you guys are OK with that.. ๐Ÿ˜…

So basically as long as it's same across components, no mater the style, you guys are OK with that.. ๐Ÿ˜…

Pretty much ๐Ÿ˜…
But for the sake of more input.. I think they both have their pros and cons!

1. Color, Alignment, Transform, Weight

  • _Pro_: Looks cleaner and more standardized without unnecessary extra-descriptive wording (personal opinion).
  • _Pro_: Color would then be consistent across Button, Paragraph, etc.
  • _Con_: However, the above pro is also an inverse con, because it could possibly cause confusion among what enum to use between entities (for example Color="TextColor.Primary" vs Color="ButtonColor.Primary").

2. TextColor, TextAlignment, TextTransform, TextWeight

  • _Pro_: As @WillianGruber mentioned, its more descriptive, which is nice for self-documenting.
  • _Con_: Possibly over descriptive? TextColor="TextColor... seems almost unnecessary.

Summary
I honestly think either option is fine!
My personal preference was option 1, but with the potential for confusion between which enums to use.. its a split decision for me ๐Ÿ˜…

My problem are just those enums. TextColor="TextColor..." is direct to me, code completion will work fine, and is faster to code.

Now taking button as an example, if you get 'Color' only, and later add 'BackgroundColor', it does not pair well, as 'Color' will be too generic. That's why I personally prefer TextColor, BackgroundColor, etc. But anyway it comes, I'll be fine with it.

This is a good point @WillianGruber.

However, CSS uses exactly that example! color is for text and background-color is for background, so in that respect, I think it should be fine for people understand... But there are most likely other areas where it could get confusing! ๐Ÿ˜…

Just some more ideas for the sake of consistency :)

Firstly, if we look at the existing Button component it is using:

  • Color="Color..."
  • Type="ButtonType..."
  • Size="ButtonSize..."

So, if we decide on:

  • TextColor="TextColor..."
  • TextAlignment="TextAlignment..."

I think we should perhaps consider updating other components (like Button) to be consistent with this idea.

Alternatively, if we decide on:

  • Color="TextColor..."
  • Alignment="TextAlignment..."

I think we should update other components consistent with this too, by breaking off their coloring (e.g. Color becomes ButtonColor, etc).

Or finally, we take a combination of two!

  • Drop the idea of TextColor enum and support the Color enum for everything.
  • Use things like ButtonType="ButtonType..." and TextAlignment="TextAlignment..." where these are not shared and need to be split.

@MitchellNZ you're reading my mind exactly. While having TextColor, TextAlignment, etc. is more descriptive I don't like having to write extra code. For me personally it doesn't look right on screen.

The problem with using Colorenum for everything instead of TextColoris that they are not exactly the same. TextColor have some values that are not present in Colorenum(Body, Muted, White, Black50, White50)

You also mentioned button as an example. This one is problematic as Color is basically used as an background color, and this behavior is the same for all frameworks.

In the end every property depends on the context where it's used. Color on button is generally considered to be set as background, on Text it's considered to be set as _font color_, on Alert again it's the background. So maybe color is not the right word for this. For example in react-bootstrap they use term variant. Maybe that's something to consider.

In the end every property depends on the context where it's used. Color on button is generally considered to be set as background, on Text it's considered to be set as font color, on Alert again it's the background.

This is also exactly what I have been thinking too.
You're right, Color is typically what you'd expect for the element type (button = background, text = font) in most component libraries.

So maybe color is not the right word for this. For example in react-bootstrap they use term variant. Maybe that's something to consider.

Another good point!
Also, Ant Design use type for _some_ of the items we are calling Color too (e.g. Link & Primary).

I think this is just a problem without a clear answer, unfortunately!
Good thing its a problem that can be solved with documentation ;)

So at the end of it, with good documentation, it probably doesn't really matter _too_ much which option we go with.
I just think its important we remain consistent and have some good rules to follow (where they make sense).

Yea I think so too. We can make a clear separation of components and put them into groups. Elements like buttons into one group, typography elements into another one, etc. And for each of the groups we make it clear in the documentation in which context the Color is applied.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

john-cullen picture john-cullen  ยท  3Comments

sroy2020 picture sroy2020  ยท  5Comments

danlbb picture danlbb  ยท  4Comments

DDDenisSobek picture DDDenisSobek  ยท  4Comments

stsrki picture stsrki  ยท  3Comments