update Clipboard #3568
update Clipboard #3568
Conversation
|
Changing the |
| /** | ||
| * The children of this component. By default copy icon | ||
| */ | ||
| children: PropTypes.node, |
There was a problem hiding this comment.
I'm a big fan of replacing the hard-coded <i> and giving users control of the child elements!
More than just the icon, I think that the default element should include the wrapper button/div itself. For example, a user might prefer to use an anchor instead.
There was a problem hiding this comment.
That seems like a good idea, however, the Clipboard is essentially a Button, (and the base element should be updated to be a Button rather than a Div). It’s invalid HTML to have a Button or other interactive element as children of a Button. The purpose of updating the children is to simply provide feedback on the copy state.
| children: PropTypes.node, | ||
|
|
||
| /** | ||
| * The children of this component displayed while copying. By default checked icon |
There was a problem hiding this comment.
| * The children of this component displayed while copying. By default checked icon | |
| * The children of this component displayed after copying. By default checked icon |
| /** | ||
| * The icon's styles while copying | ||
| */ | ||
| copied_style: PropTypes.object, | ||
|
|
||
| /** | ||
| * The class name of the icon element while copying | ||
| */ | ||
| copied_className: PropTypes.string, |
There was a problem hiding this comment.
Hmm, this seems like we're adding 2 or 3 ways of solving this problem at once. If we are allowing custom children as above, is there really any need to have these 2 style props? Users could apply their own styling to the child components and achieve the same effect with less complexity on our end.
(also, there might be good reasons here, I'm just not seeing it immediately!)
There was a problem hiding this comment.
I may have misunderstood you first comment above. Can you clarify? Did you mean that the children and copied_children could be buttons or anchors? Or were you referring to only the root element of the Clipboard?
But if children and copied_children is a div with styling, I agree that you would then not need the coied_style and copied_className
2 participants
Closes #3565
See issue for details