Enable button variations

[sandhana14]

Passing bgColor prop because we use two different bg colors in our tradedesk for the button component.

[alkiko]

I think it would be more clean to add the following props:

• primary
• secondary
• tertiary

This should cover all the buttons in the left column in the overview below.

Then we can apply the styles accordingly: e.g. <Button primary/> would apply the primary background and corresponding hover and clicked states, giving:

• <Button primary/>
• <Button secondary/>
• <Button tertiary/>

I don't recall seeing the buttons from the second column yet, but if we need them we could perhaps add an "outline" prop to the button:

• <Button primary outline/>
• <Button secondary outline/>
• tertiary outline button does not seem to exist

I'm very open to other solutions, but I think using <Button bgColor="bg-buttonPrimary"/> and <Button bgColor="bg-buttonSecondary"/> is a bit inaccurate since we're not interested in arbitrary background colour options, but rather in changing the entire button variant: primary, secondary, tertiary.

What do you think @sandhana14 and @pixochi?

[sandhana14]

I think it would be more clean to add the following props:

• primary
• secondary
• tertiary

This should cover all the buttons in the left column in the overview below.

Then we can apply the styles accordingly: e.g. <Button primary/> would apply the primary background and corresponding hover and clicked states, giving:

• <Button primary/>
• <Button secondary/>
• <Button tertiary/>

I don't recall seeing the buttons from the second column yet, but if we need them we could perhaps add an "outline" prop to the button:

• <Button primary outline/>
• <Button secondary outline/>
• tertiary outline button does not seem to exist

I'm very open to other solutions, but I think using <Button bgColor="bg-buttonPrimary"/> and <Button bgColor="bg-buttonSecondary"/> is a bit inaccurate since we're not interested in arbitrary background colour options, but rather in changing the entire button variant: primary, secondary, tertiary.

What do you think @sandhana14 and @pixochi?

I agree with you. It's nice to have primary and secondary variants. I think so we don't have buttons with outline and tertiary variant in our design. I don't think so it's needed now.

[pixochi]

What do you think @sandhana14 and @pixochi?

It's common in React UI libraries to use 2 props for styling buttons. One prop for the shape of button and another for its colors. I would propose 2 new props for the Button component: variant and color, where variant is one of ['contained', 'outlined', 'text', 'link'], and color is one of ['primary', 'secondary'] (later also possibly error, warning).

Button examples using the 2 proposed props (the design in the screenshots is approximate, only for illustrating the new props):

variant

color

Currently we would need to implement only the contained variant, and primary and secondary color(s). I think primary color and contained variant could be good defaults.

[alkiko]

Fine with me @pixochi.

My reference was https://react.semantic-ui.com/elements/button/#types-emphasis.

I'm happy with both.

[pixochi]

My reference was https://react.semantic-ui.com/elements/button/#types-emphasis.

I'm happy with both.

I'm not a fan of the interface by Semantic UI. They basically use multiple boolean values (primary, secondary ) for the type where you could run into conflicts, e.g.:

<Button primary secondary />

[alkiko]

Good point. Agreed. Let’s go with your proposal, unless @sandhana14 disagrees?

BTW I think the easy solution to semantic UI’s style is to cascade it.

[alkiko]

Could you write up examples to cover all buttons in the style guide?

As I see it we have [primary secondary tertiary] x [fill outline]

Just to be sure we’re all on the same page.

[sandhana14]

Could you write up examples to cover all buttons in the style guide?

As I see it we have [primary secondary tertiary] x [fill outline]

Just to be sure we’re all on the same page.

I think so, we have to implement in color - primary, secondary and in variant - contained, text, may be outline(I haven't seen outline button used in our design yet, but not completely sure)

[alkiko]

I just talked with the designer. She (of course) has a third suggestion 😅

variant: enum[primary, secondary, tertiary] danger: boolean (uses the red color theme - see screenshots in the bottom) outline: boolean (default: false - no effect on tertiary)

Example

<Button variant="primary"/>
<Button variant="primary" outline/>
<Button variant="primary" danger/>
<Button variant="primary" outline danger/>

<Button variant="secondary"/>
<Button variant="secondary" outline/>
<Button variant="secondary" danger/>
<Button variant="secondary" outline danger/>

<Button variant="tertiary"/>
<Button variant="tertiary" danger/>

Danger button

Future

• Later there's going to be an icon prop too, where we can pass an icon.
• There will also be the option to disable the label, so it only shows an icon.

Differences from @pixochi's suggestion

• Main difference is that she uses "variant" in stead of "color" to distinguish between primary, secondary and tertiary in the design system.
• Danger is not a color or variant - it's an entirely different button used to signal destructive actions.
• Color can be used later, e.g. to pass in hex codes for complete customisation.

[pixochi]

I just talked with the designer. She (of course) has a third suggestion sweat_smile

Who would have thought a button can be such a controversial topic :laughing:

variant: enum[primary, secondary, tertiary] danger: boolean (uses the red color theme - see screenshots in the bottom) outline: boolean (default: false - no effect on tertiary)

I think the outline prop is not good for scalability, it is just one of possible "shapes" we (might) need. A single prop responsible for the shapes could be more easily extended as opposed to having a new prop for every new shape. E.g., both Ant Design and Material UI use an enum of shapes for a single prop (type, variant).

Example with the variant and color:

<Button color="primary" variant="contained"/>
<Button color="primary" variant="outlined"/>
<Button color="primary" variant="text"/>

<Button color="secondary" variant="contained"/>
<Button color="secondary" variant="outlined"/>
<Button color="secondary" variant="text"/>

<Button color="danger" variant="contained"/>
<Button color="danger" variant="outlined"/>
<Button color="danger" variant="text"/>

[alkiko]

Who would have thought a button can be such a controversial topic 😆

Haha, yes. Good old engineers <-> designers.

I think your point about scalability are important, so we don't get potential prop conflict overflow.

Luckily I think we're down to semantics now.

I tend to agree that color isn't the most fitting to indicate the button variant: primary, secondary or tertiary, as it's not just the color that changes:

• Primary: uses the primary color
• Secondary: uses the secondary color
• Tertiary: uses the primary color but doesn't have a surrounding box

We looked into ant designs button API and it seems to cover most cases.

Inspired by that, here's a suggestion:

Property	Description	Type	Default
type	Set the original html type of button	enum (button, submit, reset)	button
danger	Indicates destructive actions	boolean	false
variant	Variant of button / prominence	enum (primary, secondary, tertiary)	primary
appearance	Each variant can have appearances	enum (default, outline)	default
shape	Buttons can have different shapes	enum (default, square)	default
size	Choose the size of the button	enum (sm,lg)	lg
icon	Buttons can show an icon	ReactNode	-
onClick	Set the onClick handler	function	-
disabled	Indicate that the button has no action	boolean	false
block	Take full width of the parent container	boolean	false
label	Text on the button	string	-
loading	Shows	boolean	false

Design notes

The color property is deliberately avoided, as it indicates freedom in color choice - this is a design choice. When choosing which button to use, we should consider:

• Prominence (variant): is this a primary, secondary or tertiary action?
• Context (type): the filled button can be too prominent on some background or layouts and dominate too much. Thus the outline option.
• Danger is passed separately, as this should be applied in both filled, outlined, primary, secondary, tertiary. It will always be red.

Allowing to choose buttons based on a color property:

• Opens up for escalation: defining new colors for convenience - the existing ones doesn't seem to "fit".
• Interaction perspective: having a few well-thought-out colors ensure clear distinction between primary, secondary and teritary buttons. Adding more colors "ad hoc" blurs this distinction for the user.

[pixochi]

Inspired by that, here's a suggestion:

Property	Description	Type	Default
htmlType	Set the original html type of button	enum (button, submit, reset)	button
danger	Indicates destructive actions	boolean	false
variant	Variant of button / prominence	enum (primary, secondary, tertiary)	primary
type	Each variant can have different types	enum (default, outline)	default

As we discussed, htmlType should be just type, and type should be called appearance.

[alkiko]

I updated the table in https://github.com/cinemataztic/cine-ui/pull/24#issuecomment-1018269611 to reflect that. .

[alkiko]

I think we need to settle on the color naming in #34 first @pixochi and @sandhana14.

[sandhana14]

I think we need to settle on the color naming in #34 first @pixochi and @sandhana14.

Yes, we have to