GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — tools: add button to copy code example to clipboard by jakecastelli · Pull Request #46928 · nodejs/node

Conversation

@jakecastelli

@jakecastelli jakecastelli commented Mar 3, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Fixes: #46894

Open the PR early for some initial feedback.

  • support js flavor (cjs, mjs)
  • support code snippet without js flavor
  • fix the button style

Quick demo:
https://user-images.githubusercontent.com/38635403/222962223-4a6e21af-4a8b-420d-8d9b-10b8a1138ea6.mov

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@nodejs-github-bot nodejs-github-bot added doc Issues and PRs related to Node.js documentation. tools Issues and PRs related to the tools directory. labels Mar 3, 2023
@jakecastelli
tools: add button to copy code example to clipboard
403718c
@jakecastelli
jakecastelli force-pushed the tool-add-button-to-copy-code-to-clipboard branch from 0e458bf to 403718c Compare March 3, 2023 08:43
@jakecastelli jakecastelli changed the title [WIP] tool: add button to copy code example to clipboard tools: add button to copy code example to clipboard Mar 3, 2023
jakecastelli added 5 commits March 3, 2023 22:51
@jakecastelli
fix: keep setup function naming consistent
d8ea97b
@jakecastelli
fix: improve copy button naming
e3bd6f2
@jakecastelli
fix: move copy button to the right
847f064
@jakecastelli
fix: improve button style
8b1fde0
@jakecastelli
fix: add indicator for copied
bb8bb12
@jakecastelli

jakecastelli commented Mar 5, 2023

Copy link
Copy Markdown
Member Author

Screenshot 2023-03-05 at 11 31 15 pm

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jakecastelli
jakecastelli marked this pull request as ready for review March 5, 2023 13:07
@debadree25

debadree25 commented Mar 6, 2023

Copy link
Copy Markdown
Contributor

cc @nodejs/website

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tniessen

tniessen commented Mar 6, 2023

Copy link
Copy Markdown
Member

@ovflowd Since doc rendering might be changed to a different mechanism, is there any chance this might be better implemented elsewhere?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ovflowd

ovflowd commented Mar 6, 2023

Copy link
Copy Markdown
Member

The generation will be done elsewhere in the future, and the tooling will be an NPM package. I believe adding a "Copy code" feature is OK for now. So as you know, this code might be discarded entirely in the future.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jakecastelli

jakecastelli commented Mar 7, 2023

Copy link
Copy Markdown
Member Author

Thank you for triaging @debadree25 @tniessen

Hi @ovflowd, I agree that the nodejs.dev would be much easier to work with as this one is really adding feature to the legacy code. I am ok with discarding it after we moved to nodejs.dev, it is just a good addition for the moment.

Thanks for letting me know earlier, so I would not plan to spend too much time on the legacy code 😄

ovflowd reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@ovflowd

ovflowd commented Mar 7, 2023 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Hi @ovflowd, I agree that the nodejs.dev would be much easier to work with as this one is really adding feature to the legacy code. I am ok with discarding it after we moved to nodejs.dev, it is just a good addition for the moment.

I never said anything about nodejs.dev; from where did you assume that? 😅 I have no idea what you mean by this going to nodejs.dev

-- edit

I think I got what you're referring to; maybe you saw the "test/playground" we're doing on https://nodejs.dev/api/; just to let you know, that's not what I meant with the new docs tooling. There's an issue on nodejs/next-10#166 that is what I was referring to.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ovflowd
ovflowd approved these changes Mar 7, 2023
View reviewed changes

@ovflowd ovflowd left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Knowing this is mostly temporary code, I'm okay with the current logic. There are ways of course of optimising this, but let's not be picky for now.

My suggestion would to expose a callback that the button itself can use instead of registering who knows how many event listeners, but that's fine, IMHO.

LGTM :shipit:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
@jakecastelli

jakecastelli commented Mar 7, 2023

Copy link
Copy Markdown
Member Author

@ovflowd after reading your previous reply, then on the side bar github explore/repositories suggest it nodejs/nodejs.dev to me 😅 I had a look and assumed that would be where the future of the nodejs doc would be, apologise that I made a wrong guess ;)

Thanks for pointing me to the right direction at nodejs/next-10#166, I shall give a read, cheers mate 👍

ovflowd reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@jakecastelli jakecastelli mentioned this pull request Mar 8, 2023
doc: copy example code to the clipboard #46894
Closed
btea
btea reviewed Mar 13, 2023
View reviewed changes
Comment thread doc/api_assets/api.js
function setupCopyButton() {
const buttons = document.querySelectorAll('.copy-button');
buttons.forEach((button) => {
button.addEventListener('click', (el) => {

@btea btea Mar 13, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

The event handler may be able to extract.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@jakecastelli jakecastelli Mar 14, 2023

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hi @btea thank you for your feedback, I am just not too sure if it is worth the effort of function extraction.

  • The code is temp and won't live in the future doc tool
  • the function is relatively short and straightforward

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@btea btea Mar 17, 2023

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Yeah, what you said makes sense. I made this point because each page of the nodejs official website has a lot of content, and some have many code blocks, so there may be many bindings of the same event, so I think it is worth extracting.

Anyway, I think this feature looks great 👍

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jakecastelli reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@jakecastelli

jakecastelli commented Mar 24, 2023

Copy link
Copy Markdown
Member Author

would you guys mind having a look at this PR again? @debadree25 @tniessen 👀

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

debadree25
debadree25 approved these changes Mar 24, 2023
View reviewed changes

@debadree25 debadree25 left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Hey, I think this is LGTM, leaving it open for a little while more before landing in case anyone would like to chime in, would land this tomorrow maybe.

also cc'ing @nodejs/website for good measure

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

jakecastelli reacted with heart emoji
All reactions
  • ❤️ 1 reaction
@debadree25 debadree25 added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label Mar 24, 2023
shanpriyan
shanpriyan approved these changes Mar 24, 2023
View reviewed changes

@shanpriyan shanpriyan left a comment

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM :shipit:

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
manishprivet
manishprivet approved these changes Mar 24, 2023
View reviewed changes
@debadree25

debadree25 commented Mar 25, 2023

Copy link
Copy Markdown
Contributor

I think there is enough approval now, landing this

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@debadree25 debadree25 added the commit-queue PRs queued for automated landing through the Commit Queue. label Mar 25, 2023
@nodejs-github-bot nodejs-github-bot added commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. and removed commit-queue PRs queued for automated landing through the Commit Queue. labels Mar 25, 2023
@nodejs-github-bot

nodejs-github-bot commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator
Commit Queue failed
- Loading data for nodejs/node/pull/46928
✔  Done loading data for nodejs/node/pull/46928
----------------------------------- PR info ------------------------------------
Title      tools: add button to copy code example to clipboard (#46928)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     jakecastelli:tool-add-button-to-copy-code-to-clipboard -> nodejs:main
Labels     doc, tools, author ready
Commits    6
 - tools: add button to copy code example to clipboard
 - fix: keep setup function naming consistent
 - fix: improve copy button naming
 - fix: move copy button to the right
 - fix: improve button style
 - fix: add indicator for copied
Committers 1
 - jakecastelli <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/46928
Refs: https://github.com/nodejs/node/issues/46894
Reviewed-By: Debadree Chatterjee 
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/46928
Refs: https://github.com/nodejs/node/issues/46894
Reviewed-By: Debadree Chatterjee 
--------------------------------------------------------------------------------
   ℹ  This PR was created on Fri, 03 Mar 2023 08:41:26 GMT
   ✔  Approvals: 1
   ✔  - Debadree Chatterjee (@debadree25): https://github.com/nodejs/node/pull/46928#pullrequestreview-1357171087
   ✔  Last GitHub CI successful
   ℹ  Green GitHub CI is sufficient
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 46928
From https://github.com/nodejs/node
 * branch                  refs/pull/46928/merge -> FETCH_HEAD
✔  Fetched commits as a64469dcf10a..bb8bb125f177
--------------------------------------------------------------------------------
[main 088bb4b4b0] tools: add button to copy code example to clipboard
 Author: jakecastelli <[email protected]>
 Date: Fri Mar 3 18:55:16 2023 +1030
 2 files changed, 32 insertions(+), 2 deletions(-)
[main 67fa1dfc9f] fix: keep setup function naming consistent
 Author: jakecastelli <[email protected]>
 Date: Fri Mar 3 22:51:30 2023 +1030
 1 file changed, 2 insertions(+), 2 deletions(-)
[main cbfd343ba5] fix: improve copy button naming
 Author: jakecastelli <[email protected]>
 Date: Fri Mar 3 22:54:25 2023 +1030
 1 file changed, 4 insertions(+), 4 deletions(-)
[main 7c8f00dd20] fix: move copy button to the right
 Author: jakecastelli <[email protected]>
 Date: Sat Mar 4 22:49:10 2023 +1030
 1 file changed, 5 insertions(+)
[main 2353ea64f9] fix: improve button style
 Author: jakecastelli <[email protected]>
 Date: Sun Mar 5 23:30:41 2023 +1030
 1 file changed, 22 insertions(+)
[main 4b1a985554] fix: add indicator for copied
 Author: jakecastelli <[email protected]>
 Date: Sun Mar 5 23:31:00 2023 +1030
 1 file changed, 6 insertions(+)
   ✔  Patches applied
There are 6 commits in the PR. Attempting autorebase.
Rebasing (2/12)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
tools: add button to copy code example to clipboard

PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee [email protected]

[detached HEAD 8e8700a770] tools: add button to copy code example to clipboard
Author: jakecastelli [email protected]
Date: Fri Mar 3 18:55:16 2023 +1030
2 files changed, 32 insertions(+), 2 deletions(-)
Rebasing (3/12)
Rebasing (4/12)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fix: keep setup function naming consistent

PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee [email protected]

[detached HEAD b457accb8b] fix: keep setup function naming consistent
Author: jakecastelli [email protected]
Date: Fri Mar 3 22:51:30 2023 +1030
1 file changed, 2 insertions(+), 2 deletions(-)
Rebasing (5/12)
Rebasing (6/12)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fix: improve copy button naming

PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee [email protected]

[detached HEAD 91e88e424d] fix: improve copy button naming
Author: jakecastelli [email protected]
Date: Fri Mar 3 22:54:25 2023 +1030
1 file changed, 4 insertions(+), 4 deletions(-)
Rebasing (7/12)
Rebasing (8/12)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fix: move copy button to the right

PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee [email protected]

[detached HEAD 0cc3e6f362] fix: move copy button to the right
Author: jakecastelli [email protected]
Date: Sat Mar 4 22:49:10 2023 +1030
1 file changed, 5 insertions(+)
Rebasing (9/12)
Rebasing (10/12)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fix: improve button style

PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee [email protected]

[detached HEAD c99de7088f] fix: improve button style
Author: jakecastelli [email protected]
Date: Sun Mar 5 23:30:41 2023 +1030
1 file changed, 22 insertions(+)
Rebasing (11/12)
Rebasing (12/12)

Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
fix: add indicator for copied

PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee [email protected]

[detached HEAD 4034f072a3] fix: add indicator for copied
Author: jakecastelli [email protected]
Date: Sun Mar 5 23:31:00 2023 +1030
1 file changed, 6 insertions(+)

Successfully rebased and updated refs/heads/main.

ℹ Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/4518347599

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@debadree25 debadree25 added commit-queue PRs queued for automated landing through the Commit Queue. commit-queue-squash PRs the Commit Queue should land as one squashed commit. and removed commit-queue-failed PRs whose Commit Queue landing failed and need manual intervention before retrying. labels Mar 25, 2023
@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Mar 25, 2023
@nodejs-github-bot
nodejs-github-bot merged commit 94ec71d into nodejs:main Mar 25, 2023
@nodejs-github-bot

nodejs-github-bot commented Mar 25, 2023

Copy link
Copy Markdown
Collaborator

Landed in 94ec71d

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@tniessen tniessen mentioned this pull request Mar 27, 2023
Button to choose module type is missing #47268
Closed
@RafaelGSS RafaelGSS mentioned this pull request Apr 3, 2023
2023-04-18, Version 20.0.0 (Current) #47381
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 5, 2023
@jakecastelli @RafaelGSS
tools: add button to copy code example to clipboard
d1efe8e
PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Apr 6, 2023
v19.9.0 proposal #47441
Merged
RafaelGSS pushed a commit that referenced this pull request Apr 6, 2023
@jakecastelli @RafaelGSS
tools: add button to copy code example to clipboard
8cba1fc
PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Apr 7, 2023
@jakecastelli @RafaelGSS
tools: add button to copy code example to clipboard
deb80b1
PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee <[email protected]>
danielleadams pushed a commit that referenced this pull request Jul 6, 2023
@jakecastelli @danielleadams
tools: add button to copy code example to clipboard
37638e4
PR-URL: #46928
Refs: #46894
Reviewed-By: Debadree Chatterjee <[email protected]>
@danielleadams danielleadams mentioned this pull request Jul 10, 2023
v18.17.0 release proposal #48694
Merged
abhishekumar-tyagi pushed a commit to abhishekumar-tyagi/node that referenced this pull request May 5, 2024
@jakecastelli
tools: add button to copy code example to clipboard
39e40a0
PR-URL: nodejs/node#46928
Refs: nodejs/node#46894
Reviewed-By: Debadree Chatterjee <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@ovflowd ovflowd ovflowd approved these changes
+4 more reviewers
@btea btea btea left review comments
@debadree25 debadree25 debadree25 approved these changes
@shanpriyan shanpriyan shanpriyan approved these changes
@manishprivet manishprivet manishprivet approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. commit-queue-squash PRs the Commit Queue should land as one squashed commit. doc Issues and PRs related to Node.js documentation. tools Issues and PRs related to the tools directory.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

doc: copy example code to the clipboard

Uh oh!

There was an error while loading. Please reload this page.

8 participants

@jakecastelli @debadree25 @tniessen @ovflowd @nodejs-github-bot @btea @shanpriyan @manishprivet
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.