Skip to content
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

Make copy-to-clipboard configurable with multiple attributes #2723

Merged
merged 8 commits into from
Jan 31, 2021
Merged

Make copy-to-clipboard configurable with multiple attributes #2723

merged 8 commits into from
Jan 31, 2021

Conversation

edukisto
Copy link
Contributor

Resolves #1438.

Makes the copy-to-clipboard plugin consider the following HTML attributes:

  • data-prismjs-copy,
  • data-prismjs-copy-error,
  • data-prismjs-copy-success,
  • data-prismjs-copy-timeout.

@github-actions
Copy link

github-actions bot commented Jan 18, 2021

JS File Size Changes (gzipped)

A total of 2 files have changed, with a combined diff of +22 B (+1.9%).

file master pull size diff % diff
components/prism-http.min.js 644 B 559 B -85 B -13.2%
plugins/copy-to-clipboard/prism-copy-to-clipboard.min.js 525 B 632 B +107 B +20.4%

Generated by 🚫 dangerJS against f1cf157

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Thank you for the PR @edukisto!

A few nits. Also, please add some documentation on how to use the feature you implemented.

plugins/copy-to-clipboard/prism-copy-to-clipboard.js Outdated Show resolved Hide resolved
plugins/copy-to-clipboard/prism-copy-to-clipboard.js Outdated Show resolved Hide resolved
plugins/copy-to-clipboard/prism-copy-to-clipboard.js Outdated Show resolved Hide resolved
@edukisto
Copy link
Contributor Author

I had to replace hyphenated-keys with underscored_keys to make keys compatible with the @param syntax.

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Please add a few words on the Copy to Clipboard plugin page to explain how to use the new localization method. Maybe make it a new section? It doesn't have to be much. Just explain that these attributes are inherited from ancestor nodes, what attributes there are, and what they do.

@edukisto
Copy link
Contributor Author

Please add a few words on the Copy to Clipboard plugin page...

Sure, I remember the first request.

Thank you very much for your advices!

Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

Very good and very detailed documentation! Yay!

A few minor nits and we're ready to go from what I see.

plugins/copy-to-clipboard/index.html Show resolved Hide resolved
plugins/copy-to-clipboard/index.html Outdated Show resolved Hide resolved
plugins/copy-to-clipboard/index.html Show resolved Hide resolved
plugins/copy-to-clipboard/index.html Outdated Show resolved Hide resolved
edukisto and others added 2 commits January 27, 2021 03:39
Co-authored-by: Michael Schmidt <mitchi5000.ms@googlemail.com>
Copy link
Member

@RunDevelopment RunDevelopment left a comment

Choose a reason for hiding this comment

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

LGTM;

@mAAdhaTTah What about you?

@mAAdhaTTah
Copy link
Member

@RunDevelopment I'm into this but need some time to review. Will try to get to it this evening.

Copy link
Member

@mAAdhaTTah mAAdhaTTah left a comment

Choose a reason for hiding this comment

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

I love this. I think we're just waiting on the verification of the Russian "Copied!" translation and then we're gtg.

@RunDevelopment
Copy link
Member

@mAAdhaTTah You think it's good to go?

@mAAdhaTTah mAAdhaTTah merged commit 2cb909e into PrismJS:master Jan 31, 2021
@mAAdhaTTah
Copy link
Member

@RunDevelopment Yup! :)

@edukisto Thank you contributing!

@edukisto edukisto deleted the topic-1438-multiple-attributes branch January 31, 2021 19:59
@edukisto
Copy link
Contributor Author

@RunDevelopment and @mAAdhaTTah, thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow copy-to-clipboard texts to be translated
3 participants