indiv-legacy / kit

https://indivorg.github.io/kit
Apache License 2.0
0 stars 0 forks source link

New LabelledButton component #11

Open braaar opened 3 years ago

braaar commented 3 years ago

Requirements for the LabelledButton component we want:

Good: Screenshot 2021-09-07 at 10 01 30

Bad: Screenshot 2021-09-07 at 10 02 00

braaar commented 3 years ago

New style:

Screenshot 2021-09-09 at 10 06 04
simenandre commented 3 years ago

Can this just be a styled Button? or maybe IconButton? I don't understand the name LabelledButton.

braaar commented 3 years ago

Very true. I like IconButton. LabelledButton is basically a tautology

braaar commented 3 years ago

I'm not sure if we will be able to pull this off with just styling on the button object. We probably need a flex to align the icons properly

simenandre commented 3 years ago

Right, let's implement a <IconButton /> where one of the inputs is an icon (ReactElement).

E.g.

<IconButton icon={<Icon />} />

Not sure about the name IconButton, considering this: https://theme-ui.com/components/icon-button/

braaar commented 3 years ago

Right, let's implement a <IconButton /> where one of the inputs is an icon (ReactElement).

E.g.

<IconButton icon={<Icon />} />

Not sure about the name IconButton, considering this: https://theme-ui.com/components/icon-button/

I like this syntax! I think it will end up being

import {Icon} from 'react-icons';

<IconButton icon={Icon} />
braaar commented 3 years ago

Brainstorming names

ButtonWithReactIcon ReactIconButton SymbolButton ActionButton

seacurrent commented 3 years ago

ActionButton gets my vote.

simenandre commented 3 years ago

I don't know, isn't all buttons meant to have an action? I'll do some research, see if find some other examples/names.

braaar commented 3 years ago

ButtonWithGraphic ButtonWithSymbol IndivButton

simenandre commented 3 years ago

How about AffixButton? As in a button with prefix and suffix, I imagine this contract:

<AffixButton suffix={<Icon />} />
<AffixButton prefix={<Icon />} />
<AffixButton prefix={<PreIcon />} suffix={<AfterIcon />} />

What do you think?

braaar commented 2 years ago

The name has its logic, I guess, but it's not intuitive at a glance, in my opinion. Something referring to the fact that the button has an icon, graphic, visual element on it is more intuitive.

I noticed a bug in the current implementation. There is a small margin on the text label which does not trigger the onClick action and also does not show the mouse as a cursor.

https://user-images.githubusercontent.com/5765650/134669788-697cc2e3-8346-4608-8f75-01bb2e0d3627.mov

braaar commented 2 years ago

Actually, the entire text area does not trigger the onClick.