Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Added button that copies code to clipboard #1040

Merged
merged 2 commits into from
Jun 8, 2017
Merged

Added button that copies code to clipboard #1040

merged 2 commits into from
Jun 8, 2017

Conversation

ollieh
Copy link
Contributor

@ollieh ollieh commented Jun 5, 2017

@matrixbot
Copy link
Member

Can one of the admins verify this patch?

@ollieh ollieh changed the base branch from master to develop June 5, 2017 23:41
};
}
}, 10);
}
Copy link
Member

Choose a reason for hiding this comment

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

This looks like the right approach, although I don't understand why you're adding the onclick handlers asynchronously afterwards here. You've also copied the comment from the block above, but it's not relevant to this block and quite misleading.

Copy link
Member

Choose a reason for hiding this comment

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

It took me a while to realise that of course you can't add the onclick handlers here because you're just using it to parse, manipulate and re-generate the HTML markup. I'm going to add a comment to make it a bit easier to understand what's going on here, but otherwise this looks good.

src/HtmlUtils.js Outdated
@@ -363,6 +364,18 @@ export function bodyToHtml(content, highlights, opts) {
return <span className={className} dangerouslySetInnerHTML={{ __html: safeBody }} />;
}

function addCodeCopyButton(safeBody) {
var el = document.createElement("div");
Copy link
Member

Choose a reason for hiding this comment

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

We should never we writing var in new code

ollieh added 2 commits June 6, 2017 12:33
Signed-off-by: Oliver Hunt <oliver@hunt.bz>
Signed-off-by: Oliver Hunt <oliver@hunt.bz>
@@ -80,6 +93,15 @@ module.exports = React.createClass({
}
}, 10);
}
const buttons = ReactDOM.findDOMNode(this).getElementsByClassName("mx_EventTile_copyButton");
if (buttons.length > 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Also this check is now redundant, since if the length is 0, the for loop will just run zero times.

@dbkr dbkr merged commit b40636a into matrix-org:develop Jun 8, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants