Skip to content

Conversation

@cnaples79
Copy link

@cnaples79 cnaples79 commented Dec 26, 2025

Summary

  • replace toast-based copy confirmation with inline feedback on the code block button
  • reuse the existing copy hook to toggle button text/icon to "Copied" after clicking

Rationale

  • keeps feedback adjacent to the action and removes the global toast dependency (accessibility and clarity)

Changes

  • CodeBox now uses useCopyToClipboard state for inline status and no longer calls NotificationProvider
  • BaseCodeBox accepts an optional copied flag to swap the copy icon for a checkmark

Fixes #8357

@cnaples79 cnaples79 requested a review from a team as a code owner December 26, 2025 01:02
@vercel
Copy link

vercel bot commented Dec 26, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Review Updated (UTC)
nodejs-org Ready Ready Preview Dec 29, 2025 4:03pm

@github-actions
Copy link
Contributor

👋 Codeowner Review Request

The following codeowners have been identified for the changed files:

Team reviewers: @nodejs/nodejs-website

Please review the changes when you have a chance. Thank you! 🙏

@codecov
Copy link

codecov bot commented Dec 26, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 74.28%. Comparing base (732de04) to head (d1877ca).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #8458      +/-   ##
==========================================
+ Coverage   73.69%   74.28%   +0.59%     
==========================================
  Files         108      106       -2     
  Lines        9210     9112      -98     
  Branches      313      307       -6     
==========================================
- Hits         6787     6769      -18     
+ Misses       2421     2341      -80     
  Partials        2        2              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@mikeesto mikeesto left a comment

Choose a reason for hiding this comment

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

Looks good, thanks! Could you please run prettier on apps/site/components/Common/CodeBox.tsx to fix CI?

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

I believe you can remove the whole source of NotificationProvider, including tests, stories and remove it from the Layout too. Which would also allow you to remove the "use client" from the layout. (I think?)

@avivkeller
Copy link
Member

Hi @cnaples79! Thanks for the PR, let me know if you need help with @ovflowd's request above, i'd be happy to push a commit helping :-)

@avivkeller avivkeller self-assigned this Dec 28, 2025
Co-Authored-By: cnaples79 <cnaples79@gmail.com>
Signed-off-by: Aviv Keller <me@aviv.sh>
@github-actions github-actions bot removed the github_actions:pull-request Trigger Pull Request Checks label Dec 29, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Dec 29, 2025

Lighthouse Results

URL Performance Accessibility Best Practices SEO Report
/en 🟢 98 🟠 87 🟢 100 🟢 100 🔗
/en/about 🟢 100 🟢 93 🟢 100 🟠 88 🔗
/en/about/previous-releases 🟢 99 🟢 93 🟢 100 🟢 100 🔗
/en/download 🟢 97 🟢 96 🟢 100 🟢 100 🔗
/en/download/archive/current 🟢 100 🟢 100 🟢 100 🟢 100 🔗
/en/blog 🟢 99 🟢 100 🟢 96 🟢 100 🔗

</>
) : (
<>
<CodeBracketIcon className="size-4" />
Copy link
Member

Choose a reason for hiding this comment

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

What is this icon?

Also nit: (Cleared and less code)

const ButtonIcon = copied ? DocumentDuplicateIcon : CodeBracketIcon;

...

buttonContext={
  <>
    <ButtonIcon className="size-4" />
    {t(copied ? 
        'components.common.codebox.copied' :
        'components.common.codebox.copy'
    )}
  </>
}

Copy link
Member

Choose a reason for hiding this comment

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

What is this icon?

What do you mean? This is the icon we used previously, is it not?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I realized that afterwards.

codeLanguage
);

// Adds a Copy Button to the CodeBox if requested as an additional parameter
Copy link
Member

Choose a reason for hiding this comment

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

🤔

Copy link
Member

@ovflowd ovflowd left a comment

Choose a reason for hiding this comment

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

These changes diverge so much from the original PR provided by OP that these should be a new PR. IMO, with how distinct HEAD is now from the previous commit, this completely removes the "co-author" factor from the original PR author.

@avivkeller
Copy link
Member

These changes diverge so much from the original PR provided by OP that these should be a new PR. IMO, with how distinct HEAD is now from the previous commit, this completely removes the "co-author" factor from the original PR author.

Sorry! I meant to help OP expand their PR by removing the NotificationProvider + fixup some Code box changes, I definitely did not mean to overtake this PR 😓

@ovflowd
Copy link
Member

ovflowd commented Dec 29, 2025

These changes diverge so much from the original PR provided by OP that these should be a new PR. IMO, with how distinct HEAD is now from the previous commit, this completely removes the "co-author" factor from the original PR author.

Sorry! I meant to help OP expand their PR by removing the NotificationProvider + fixup some Code box changes, I definitely did not mean to overtake this PR 😓

It's fine to address some changes, but the extra additions completely overtake the PR, it's "fine", I do believe OP abandoned the PR (possibly), but please make a new PR. (and if possible revert your commit in this PR), then we approve yours, merge and close this one.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Replace Toast notifications with inline feedback for "Code Copied" action

4 participants