-
Notifications
You must be signed in to change notification settings - Fork 0
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
New LabelledButton component #11
Comments
Can this just be a styled |
Very true. I like IconButton. LabelledButton is basically a tautology |
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 |
Right, let's implement a E.g. <IconButton icon={<Icon />} /> Not sure about the name |
I like this syntax! I think it will end up being import {Icon} from 'react-icons';
<IconButton icon={Icon} /> |
Brainstorming names
|
ActionButton gets my vote. |
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. |
|
How about <AffixButton suffix={<Icon />} />
<AffixButton prefix={<Icon />} />
<AffixButton prefix={<PreIcon />} suffix={<AfterIcon />} /> What do you think? |
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. Screen.Recording.2021-09-24.at.14.49.57.mov |
Actually, the entire text area does not trigger the onClick. |
Requirements for the LabelledButton component we want:
Good:
Bad:
The text was updated successfully, but these errors were encountered: