feat(ui): inline code block feedback #8493
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
👋 Codeowner Review RequestThe following codeowners have been identified for the changed files: Team reviewers: @nodejs/nodejs-website @nodejs/web-infra Please review the changes when you have a chance. Thank you! 🙏 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8493 +/- ##
==========================================
+ Coverage 73.70% 74.27% +0.57%
==========================================
Files 108 106 -2
Lines 9210 9112 -98
Branches 313 307 -6
==========================================
- Hits 6788 6768 -20
+ Misses 2420 2342 -78
Partials 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the code feedback mechanism by removing the global NotificationProvider toast system in favor of inline feedback within code blocks. The copy button now shows immediate visual feedback by changing its icon and text directly, eliminating the need for a separate notification system.
- Removes
NotificationProviderandNotificationcomponents along with the@radix-ui/react-toastdependency - Refactors
BaseCodeBoxAPI to acceptbuttonContent(ReactNode) instead ofbuttonText(string), enabling dynamic button content - Updates
CodeBoxto useuseCopyToClipboardhook for inline state management and conditional button rendering
Reviewed changes
Copilot reviewed 17 out of 18 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Removes @radix-ui/react-toast dependency from lockfile |
| packages/ui-components/src/Providers/NotificationProvider/index.tsx | Deletes NotificationProvider context and component |
| packages/ui-components/src/Providers/NotificationProvider/index.module.css | Deletes NotificationProvider styles |
| packages/ui-components/src/Providers/NotificationProvider/tests/index.test.jsx | Deletes NotificationProvider tests |
| packages/ui-components/src/Common/Notification/index.tsx | Deletes Notification component |
| packages/ui-components/src/Common/Notification/index.stories.tsx | Deletes Notification stories |
| packages/ui-components/src/Common/Notification/index.module.css | Deletes Notification styles |
| packages/ui-components/src/Common/CodeTabs/index.stories.tsx | Updates story to use new buttonContent prop |
| packages/ui-components/src/Common/BaseCodeBox/index.tsx | Changes API from buttonText to buttonContent, removes showCopyButton prop, adds unused copied prop |
| packages/ui-components/src/Common/BaseCodeBox/index.stories.tsx | Removes story variant, missing required props for updated API |
| packages/ui-components/src/Common/BaseCodeBox/index.module.css | Removes unused icon styles |
| packages/ui-components/package.json | Bumps version to 1.5.0 and removes @radix-ui/react-toast dependency |
| packages/ui-components/.storybook/preview.tsx | Removes NotificationProvider decorator from Storybook |
| packages/rehype-shiki/src/plugin.mjs | Removes showCopyButton metadata handling |
| apps/site/next.config.mjs | Removes @radix-ui/react-toast from transpilation config |
| apps/site/layouts/Base.tsx | Removes NotificationProvider wrapper and 'use client' directive |
| apps/site/components/MDX/CodeBox/index.tsx | Removes showCopyButton prop handling |
| apps/site/components/Common/CodeBox.tsx | Implements inline feedback using useCopyToClipboard hook with conditional icon/text rendering |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
📦 Build Size ComparisonSummary
Changes➕ Added Assets (12)
➖ Removed Assets (12)
|
Interestingly the bundle became bigger? |
|
Now we have a funny situation, technically speaking these changes (some of them) come from another PR that has been there for days (also the original commit comes from a commit you authored on that same PR). So technically speaking this isn't a fresh PR. @nodejs/nodejs-website do y'all agree into fast-tracking this? This PR can also sit and wait. |
Closes #8458
Fixes #8357
This PR removes the
NotificationProviderin favor of inline feedback, and removes the un-usedshowCopyButtonproperty fromCodeBox