Skip to content

update Clipboard #3568

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.

Sign up for GitHub

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

Draft
AnnMarieW wants to merge 2 commits into
base: dev
Choose a base branch
from
+38 −7
Draft

update Clipboard #3568

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6dbafd5
update Clipboard
AnnMarieW Jan 12, 2026
4a3d6ab
Keep div as element type so it doesn't change the layout
AnnMarieW Jan 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 38 additions & 7 deletions components/dash-core-components/src/components/Clipboard.react.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,20 +128,32 @@ export default class Clipboard extends React.Component {
}

render() {
const {id, title, className, style} = this.props;
const copyIcon = <FontAwesomeIcon icon={faCopy} />;
const copiedIcon = <FontAwesomeIcon icon={faCheckCircle} />;
const btnIcon = this.state.copied ? copiedIcon : copyIcon;
const {
id,
title,
className,
style,
copied_className,
copied_style,
children,
copied_children,
} = this.props;

const isCopied = this.state.copied;

const content = isCopied
? copied_children ?? <FontAwesomeIcon icon={faCheckCircle} />
: children ?? <FontAwesomeIcon icon={faCopy} />;

return clipboardAPI ? (
<LoadingElement
id={id}
title={title}
style={style}
className={className}
style={isCopied ? copied_style ?? style : style}
className={isCopied ? copied_className ?? className : className}
onClick={this.onClickHandler}
>
<i> {btnIcon}</i>
{content}
</LoadingElement>
) : null;
}
Expand All @@ -160,6 +172,16 @@ Clipboard.propTypes = {
*/
id: PropTypes.string,

/**
* The children of this component. By default copy icon
*/
children: PropTypes.node,
Comment on lines +175 to +178
Copy link
Contributor

@KoolADE85 KoolADE85 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Collaborator Author

@AnnMarieW AnnMarieW Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


/**
* The children of this component displayed while copying. By default checked icon
Copy link
Contributor

@KoolADE85 KoolADE85 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
* The children of this component displayed while copying. By default checked icon
* The children of this component displayed after copying. By default checked icon

*/
copied_children: PropTypes.node,

/**
* The id of target component containing text to copy to the clipboard.
* The inner text of the `children` prop will be copied to the clipboard. If none, then the text from the
Expand Down Expand Up @@ -196,6 +218,15 @@ Clipboard.propTypes = {
* The class name of the icon element
*/
className: PropTypes.string,
/**
* The icon's styles while copying
*/
copied_style: PropTypes.object,

/**
* The class name of the icon element while copying
*/
copied_className: PropTypes.string,
Comment on lines +221 to +229
Copy link
Contributor

@KoolADE85 KoolADE85 Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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!)

Copy link
Collaborator Author

@AnnMarieW AnnMarieW Jan 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@KoolADE85 KoolADE85 Jan 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was thinking the base element of the clipboard could remain a <div> and the default children could be

<button>
  <Icon />
</button>

As you note, if we eventually make the base element a button, then there are certain child components that become invalid (and this would not be obvious to Python users).
If, instead, we make the default child a button, then users can safely replace that with any Dash component, including a customized button.


/**
* Dash-assigned callback that gets fired when the value changes.
Expand Down