✨ Bento `amp-copy` #35221

mangesh · 2021-07-14T14:17:42Z

The goal of the PR is to support copying to the clipboard functionality.

Facilitating clipboard copying is useful not only for AMP.dev but also for any site that has code samples.

Features

Copy any content to the clipboard
- Support for copying value of input tags (including textarea)
- Support for copying text content inside any HTML tag (e.g div, code, pre, p tags)
Button Customisation
- Any HTML tag or plain text can be used inside the amp-copy button

In Progress

Styling
Test cases

Output/Demo

Fixes #4853

adamsilverstein · 2021-07-14T15:08:53Z

extensions/amp-copy/1.0/component.js

+export function Copy({children, sourceId, text, ...rest}) {
+  const [status, setStaus] = useState(null);
+  let theme = useMemo(()=>{
+    console.log('theme');


Are these console logging lines for debugging?

I will get this removed.

adamsilverstein · 2021-07-14T15:10:10Z

extensions/amp-copy/1.0/component.js

+    const content = document.getElementById(sourceId);
+    let text;
+    if(content.value != undefined){
+      text = content.value.trim();


I assume trimming is the desired behavior here so the user doesn't get spaces from the beginning/end of the input?

amp-owners-bot · 2021-07-20T22:06:13Z

Hey @ampproject/wg-caching! These files were changed:

extensions/amp-copy/1.0/test/validator-amp-copy.html
extensions/amp-copy/validator-amp-copy.protoascii

Gregable · 2021-07-20T22:17:22Z

As this is a new component, has this been through design review approval yet?

mangesh · 2021-07-21T16:21:38Z

As this is a new component, has this been through design review approval yet?

It hasn’t been through the design review process yet. Will now follow the process given here and try to have this reviewed during the next meet. In the meantime, please let me know if there’s anything that needs to be updated/added for this component.

caroqliu

Thanks for making this contribution! I had a few questions and suggestions, and things are looking on the right track. In the mean time, it would be really helpful to have this go to design review with a concrete API (attributes, events, and actions, if any) so the wider community can discuss.

build-system/compile/bundles.config.extensions.json

extensions/amp-copy/1.0/amp-copy.js

extensions/amp-copy/1.0/component.js

extensions/amp-copy/1.0/base-element.js

extensions/amp-copy/1.0/component.js

caroqliu · 2021-07-22T16:16:24Z

extensions/amp-copy/1.0/component.js

+      text = content.textContent.trim();
+    }
+    if (copyTextToClipboard(window, text)) {
+      setStaus(true);


After the first successful copy, subsequent successful copies won't cause an additional render. This is because when status === true, setStatus(true) => noop. Is that intended?

Thank you for the suggestion. It wasn't intended. We have just added the rendering support for the subsequent successful copies.

Remove unwanted if and else condition

Fix some typos Optimise the code

It is not valid AMP until the feature is ready for wider use. Files can be added back later when the AMP is ready

Also, prettify the code

Text passed in text attribute will get copied, if sorceId attribute is missing

caroqliu · 2021-07-28T17:08:39Z

It looks like CircleCI wants to see a fresh pull from main in this PR to run tests: https://app.circleci.com/pipelines/github/ampproject/amphtml/13781/workflows/35ce9398-42a7-47ed-9cbc-bdcb8092f96f/jobs/226709

Gregable · 2021-07-28T17:32:06Z

It looks like the validator rules have been removed from the PR for now. Once this has been design reviewed, please ping @wg-caching for validation review on whichever PR ultimately includes the validation rules.

mangesh · 2021-07-29T23:33:30Z

As this is a new component, has this been through design review approval yet?

Quick update: I have created I2I issue/doc here for wider discussion. Please let me know if I have missed any details in that issue.

samouri · 2021-08-19T18:21:11Z

extensions/amp-copy/1.0/amp-copy.css

@@ -0,0 +1,15 @@
+/**


You can remove the copyright header from all new files now :)

samouri · 2021-08-19T18:26:12Z

extensions/amp-copy/1.0/component.js

+    if (!ref.current) {
+      return;
+    }
+    if (isCopyingToClipboardSupported(ref.current.ownerDocument)) {


Would setting isCopySupported be better suited with a callback ref? https://reactjs.org/docs/refs-and-the-dom.html#callback-refs

samouri · 2021-08-19T18:28:46Z

extensions/amp-copy/1.0/component.js

+    ref,
+    () =>
+      /** @type {!CopyDef.CopyApi} */ ({
+        copyToClipboard: (selector = null, staticText = null) => {


What should be the behavior if both are provided? Should we emit an error message?

samouri · 2021-08-19T18:29:37Z

extensions/amp-copy/1.0/component.js

+    [ref, text]
+  );
+
+  const copyText = useCallback((textToCopy) => {


Wasn't copy written to handle both the copyText and copySelector case?

samouri · 2021-08-19T18:30:44Z

extensions/amp-copy/1.0/component.js

+      ref={ref}
+      className={objstr({
+        [classes.success]: status,
+        [classes.failed]: status === false,


Suggested change

[classes.failed]: status === false,

[classes.failed]: !status,

samouri · 2021-08-19T18:32:26Z

extensions/amp-copy/1.0/component.js

+ * @param {{current: ?CopyDef.CopyApi}} ref
+ * @return {PreactDef.Renderable}
+ */
+export function CopyWithRef({children, sourceId, text, ...rest}, ref) {


nit: it may be worth aliasing this to textProp so that you can then use the variable name text in smaller scope locations instead of staticText etc.

samouri · 2021-08-19T18:32:56Z

extensions/amp-copy/amp-copy.md

+  * Remove this comment!
+-->
+
+<!--


can remove this copyright header!

mangesh · 2021-08-25T11:38:30Z

@samouri Thank you for the feedback. I am already working on this. Along with this, I am working on a similar AMP Global action, which is being discussed here.

stale · 2022-09-21T00:31:01Z

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.

mangesh and others added 3 commits July 14, 2021 18:51

Initial commit for amp-copy bento component

a19ce38

Merge branch 'ampproject:main' into bento/amp-copy

66c6e3c

♻️ Updated and prettify storybook

f988e38

adamsilverstein reviewed Jul 14, 2021

View reviewed changes

westonruter mentioned this pull request Jul 14, 2021

Add a copy link in the code on the front end westonruter/syntax-highlighting-code-block#192

Open

mangesh added 2 commits July 15, 2021 02:07

Remove debug statements

8678710

Remove CSS

4477492

mangesh marked this pull request as ready for review July 20, 2021 22:06

amp-owners-bot bot requested a review from gmajoulet July 20, 2021 22:06

Gregable added the WG: caching label Jul 20, 2021

Gregable self-requested a review July 20, 2021 22:17

caroqliu self-requested a review July 21, 2021 20:36

caroqliu requested changes Jul 22, 2021

View reviewed changes

mangesh added 9 commits July 28, 2021 19:51

Add new line

f52830f

Remove unwanted code line

5476eec

Optimise the code

58461e8

Remove unwanted if and else condition

Address the feedback

7582105

Fix some typos Optimise the code

Remove validator files

53d124e

It is not valid AMP until the feature is ready for wider use. Files can be added back later when the AMP is ready

Store isCopySupported in state based on the document value

6df6823

Also, prettify the code

Add missing dependencies

3eef1d2

Prefer ref.current.ownerDocument to document

1f15039

Add support for copying static text

553b257

Text passed in text attribute will get copied, if sorceId attribute is missing

Gregable added the WG: Caching: Triaged Initial triage from wg-caching complete. Remove label if new input required. label Jul 28, 2021

Add rendering support for the subsequent successful copies

b36e342

AnuragVasanwala and others added 4 commits August 5, 2021 20:56

Merge branch 'ampproject:main' into bento/amp-copy

4d03ab8

Register API action for copy function

96d2637

♻️ ✨ Added Action API copyToClipboard

e42c1b2

🐛 Minor Fixes for API Action

f083298

samouri reviewed Aug 19, 2021

View reviewed changes

stale bot added the Stale Inactive for one year or more label Sep 21, 2022

stale bot closed this Oct 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

✨ Bento `amp-copy` #35221

✨ Bento `amp-copy` #35221

mangesh commented Jul 14, 2021

adamsilverstein Jul 14, 2021

mangesh Jul 14, 2021

adamsilverstein Jul 14, 2021

mangesh Jul 14, 2021

amp-owners-bot bot commented Jul 20, 2021 •

edited by ampprojectbot

Loading

Gregable commented Jul 20, 2021

mangesh commented Jul 21, 2021

caroqliu left a comment

caroqliu Jul 22, 2021

mangesh Jul 28, 2021

caroqliu commented Jul 28, 2021

Gregable commented Jul 28, 2021

mangesh commented Jul 29, 2021

samouri Aug 19, 2021

samouri Aug 19, 2021

samouri Aug 19, 2021

samouri Aug 19, 2021

samouri Aug 19, 2021

samouri Aug 19, 2021

samouri Aug 19, 2021

mangesh commented Aug 25, 2021

stale bot commented Sep 21, 2022

	[classes.failed]: status === false,
	[classes.failed]: !status,

✨ Bento amp-copy #35221

✨ Bento amp-copy #35221

Conversation

mangesh commented Jul 14, 2021

Features

In Progress

Output/Demo

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

amp-owners-bot bot commented Jul 20, 2021 • edited by ampprojectbot Loading

Gregable commented Jul 20, 2021

mangesh commented Jul 21, 2021

caroqliu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

caroqliu commented Jul 28, 2021

Gregable commented Jul 28, 2021

mangesh commented Jul 29, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mangesh commented Aug 25, 2021

stale bot commented Sep 21, 2022

✨ Bento `amp-copy` #35221

✨ Bento `amp-copy` #35221

amp-owners-bot bot commented Jul 20, 2021 •

edited by ampprojectbot

Loading