feat: Add file action button group#118
Conversation
|
@Wxh16144 is attempting to deploy a commit to the cnpmweb Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds a FileActions feature using a zustand store plus UI (Raw/Copy/Download), integrates store updates from CodeViewer, conditionally renders FileActions in CustomTabs, and adjusts useFileContent to also return the resolved file URI. Changes
Sequence DiagramsequenceDiagram
participant User
participant CodeViewer
participant Store as FileActionsStore
participant CustomTabs
participant FileActionsComp as FileActions
User->>CodeViewer: open file / navigate
CodeViewer->>CodeViewer: useFileContent fetches code and fileUri
Note right of CodeViewer: useEffect triggers on code/fileUri
CodeViewer->>Store: setStore({ rawUrl: fileUri, fileContent: code })
Store->>CustomTabs: store state available
CustomTabs->>CustomTabs: activeTab === "files" ?
CustomTabs->>FileActionsComp: render FileActions (conditional)
User->>FileActionsComp: click Copy / Download / Raw
alt Copy
FileActionsComp->>User: copy fileContent to clipboard
else Download
FileActionsComp->>User: create blob, trigger download
else View Raw
FileActionsComp->>User: open rawUrl in new tab
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20–25 minutes Areas requiring extra attention:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @Wxh16144, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 此拉取请求为 npmmirror 网页界面引入了一项新功能，即在文件预览页面添加了一组文件操作按钮（原始文件、复制、下载）。这一增强功能显著简化了开发人员访问和使用预览包文件 CDN 链接的过程，将多步手动操作转变为直接的一键式操作。实现上利用 zustand 进行高效的状态管理，以便将文件数据传递给新的操作组件。 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
| useEffect(() => { | ||
| FileActions.setStore({ | ||
| rawUrl: fileUri, | ||
| fileContent: code | ||
| }) | ||
| }, [fileUri, code]) |
There was a problem hiding this comment.
这个 useEffect 在组件挂载和更新时向全局 store 注入了文件信息，但缺少了在组件卸载时进行清理的逻辑。这可能会导致在用户离开文件预览页面后，全局 store 中仍然残留着旧的文件数据，造成状态污染。建议添加一个 effect 的清理函数，在组件卸载时调用 FileActions.restoreStore() 来重置状态。
| useEffect(() => { | |
| FileActions.setStore({ | |
| rawUrl: fileUri, | |
| fileContent: code | |
| }) | |
| }, [fileUri, code]) | |
| useEffect(() => { | |
| FileActions.setStore({ | |
| rawUrl: fileUri, | |
| fileContent: code | |
| }) | |
| return () => { | |
| FileActions.restoreStore(); | |
| } | |
| }, [fileUri, code]) |
| } | ||
|
|
||
| // ========== render ========= | ||
| if (Object.values(state).some(isEmpty)) return null |
There was a problem hiding this comment.
当前的渲染条件 Object.values(state).some(isEmpty) 存在一个问题：lodash 的 isEmpty('') 会返回 true。这意味着当一个文件内容为空字符串时，fileContent 为 ''，此条件会判断为真，导致操作按钮组不被渲染。用户应该能够对空文件进行“查看原文”或“下载”等操作。建议修改此条件，仅在数据真正加载中（例如 fileContent 为 undefined）时才隐藏按钮。
| if (Object.values(state).some(isEmpty)) return null | |
| if (!state.rawUrl || state.fileContent === undefined) return null |
src/components/FileActions.tsx
Outdated
| export const FileActions = Object.assign(_FileActions, { | ||
| setStore, | ||
| restoreStore, | ||
| }); |
There was a problem hiding this comment.
通过 Object.assign 将 store 的 action (setStore, restoreStore) 作为静态属性附加到 React 组件上的模式不太常见。这种做法将状态管理逻辑和 UI 组件耦合在了一起，可能会降低代码的可维护性，也不符合社区中常见的 React 和 Zustand 的使用模式。为了更好的关注点分离，建议将 Zustand store 及其相关的 actions 移至一个单独的文件中（例如 src/store/fileActions.ts），或者通过自定义 hook 导出。考虑到你提到这是临时方案，这可以作为一个未来的改进方向。
There was a problem hiding this comment.
故意的，否则别人不知道这个 setStore 是哪里来的
There was a problem hiding this comment.
Pull request overview
This PR adds a file action button group (Raw, Copy, Download) to the package file viewer, inspired by GitHub's file interface. The implementation uses Zustand for state management to share file information between components, making it easier for users to access file URLs without navigating through browser DevTools.
Key changes:
- New FileActions component with three action buttons for viewing raw files, copying content, and downloading files
- Zustand state management integration to pass file information from CodeViewer to FileActions
- Modified useFileContent hook to return both SWR data and file URI
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/FileActions.tsx | New component implementing Raw/Copy/Download buttons with Zustand store for state management |
| src/components/CodeViewer.tsx | Injects file content and URI into FileActions store via useEffect hook |
| src/components/CustomTabs.tsx | Integrates FileActions component into the tabs' extra content area for the 'files' tab |
| src/hooks/useFile.ts | Modified to return file URI alongside SWR data for use by FileActions component |
| src/components/NPMVersionSelect.tsx | Minor formatting changes removing Space wrapper and adjusting returns |
| package.json | Adds zustand@^5.0.9 dependency for state management |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| </Space> | ||
| ) : null; | ||
| </> | ||
| ) |
There was a problem hiding this comment.
Missing semicolon at the end of the return statement. For consistency with the rest of the codebase, add a semicolon.
| }) | ||
| }, [fileUri, code]) | ||
|
|
||
|
|
There was a problem hiding this comment.
Extra blank line should be removed for consistency with the rest of the codebase formatting.
|
|
||
| function handleDownloadRaw() { | ||
| if (!state.fileContent) return; | ||
| const blob = new Blob([state.fileContent], { type: 'text/plain' }); |
There was a problem hiding this comment.
The MIME type is hardcoded as 'text/plain' for all files. This may cause issues for binary files or files with specific content types (e.g., JSON, CSS, JavaScript). Consider detecting the appropriate MIME type based on the file extension or using a more generic type like 'application/octet-stream' for downloads.
| FileActions.setStore({ | ||
| rawUrl: fileUri, | ||
| fileContent: code | ||
| }) | ||
| }, [fileUri, code]) | ||
|
|
||
|
|
There was a problem hiding this comment.
The store update in the useEffect hook will cause unnecessary updates even when the component is not viewing a file. When a file is deselected (selectedFile becomes undefined), the store still retains the old file information. Consider calling FileActions.restoreStore() in a cleanup function to reset the state when the component unmounts or when there's no selected file.
| FileActions.setStore({ | |
| rawUrl: fileUri, | |
| fileContent: code | |
| }) | |
| }, [fileUri, code]) | |
| if (!selectedFile) { | |
| FileActions.restoreStore(); | |
| return; | |
| } | |
| FileActions.setStore({ | |
| rawUrl: fileUri, | |
| fileContent: code, | |
| }); | |
| return () => { | |
| FileActions.restoreStore(); | |
| }; | |
| }, [fileUri, code, selectedFile]); |
| const url = window.URL.createObjectURL(blob); | ||
| const link = document.createElement('a'); | ||
| link.href = url; | ||
| link.download = state.rawUrl.split('/').pop() || 'file'; |
There was a problem hiding this comment.
The filename extraction logic using split('/').pop() will fail for URLs that don't end with a filename (e.g., if the URL ends with a slash). Additionally, if the path contains query parameters or fragments, they will be included in the filename. Consider extracting the filename from the full path including handling edge cases like trailing slashes and URL parameters.
| link.download = state.rawUrl.split('/').pop() || 'file'; | |
| let filename = 'file'; | |
| try { | |
| const parsedUrl = new URL(state.rawUrl, window.location.href); | |
| const segments = parsedUrl.pathname.split('/').filter(Boolean); | |
| const lastSegment = segments[segments.length - 1]; | |
| if (lastSegment) { | |
| filename = decodeURIComponent(lastSegment); | |
| } | |
| } catch { | |
| // If URL parsing fails, keep the default filename | |
| } | |
| link.download = filename; |
| function handleCopyRaw() { | ||
| if (!state.fileContent) return; | ||
| navigator.clipboard.writeText(state.fileContent); | ||
| message.success('Copied to clipboard'); |
There was a problem hiding this comment.
The clipboard write operation using navigator.clipboard.writeText() may fail if the user hasn't granted clipboard permissions or if the page is not in a secure context. Add error handling to catch potential failures and inform the user appropriately.
| function handleCopyRaw() { | |
| if (!state.fileContent) return; | |
| navigator.clipboard.writeText(state.fileContent); | |
| message.success('Copied to clipboard'); | |
| async function handleCopyRaw() { | |
| if (!state.fileContent) return; | |
| if (!navigator || !navigator.clipboard || typeof navigator.clipboard.writeText !== 'function') { | |
| message.error('Clipboard is not available in this browser or context.'); | |
| return; | |
| } | |
| try { | |
| await navigator.clipboard.writeText(state.fileContent); | |
| message.success('Copied to clipboard'); | |
| } catch (error) { | |
| // Clipboard write can fail if permissions are denied or in insecure contexts | |
| // eslint-disable-next-line no-console | |
| console.error('Failed to write to clipboard:', error); | |
| message.error('Failed to copy to clipboard. Please check your browser permissions or try again.'); | |
| } |
| } | ||
|
|
||
| // ========== render ========= | ||
| if (Object.values(state).some(isEmpty)) return null |
There was a problem hiding this comment.
The visibility check using Object.values(state).some(isEmpty) is fragile and could lead to unexpected behavior. If any state value is legitimately an empty string (like an empty file), the component would hide. Consider explicitly checking for null/undefined states or using a more specific condition like !state.rawUrl || state.fileContent === undefined.
| return !isEmpty(targetOptions) ? ( | ||
| <Space style={{ paddingRight: 32 }}> | ||
| // ========== render ========== | ||
| if (isEmpty(targetOptions)) return null |
There was a problem hiding this comment.
Missing semicolon at the end of the return statement. While JavaScript has automatic semicolon insertion, it's best practice to maintain consistency with the rest of the codebase which uses semicolons.
| if (isEmpty(targetOptions)) return null | |
| if (isEmpty(targetOptions)) return null; |
| @@ -1,9 +1,10 @@ | |||
| 'use client'; | |||
| import { Tabs } from 'antd'; | |||
| import { Flex, Space, Tabs } from 'antd'; | |||
There was a problem hiding this comment.
Unused import Space.
| import { Flex, Space, Tabs } from 'antd'; | |
| import { Flex, Tabs } from 'antd'; |
src/components/CodeViewer.tsx
Outdated
| }) | ||
| }, [fileUri, code]) |
There was a problem hiding this comment.
Avoid automated semicolon insertion (91% of all statements in the enclosing function have an explicit semicolon).
| }) | |
| }, [fileUri, code]) | |
| }); | |
| }, [fileUri, code]); |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/NPMVersionSelect.tsx (1)
136-141: Remove duplicate isEmpty check.Line 141 duplicates the check already performed on line 136. Since line 136 returns early when
isEmpty(targetOptions)is true, the check on line 141 is unreachable dead code.🔎 Proposed fix
if (isEmpty(targetOptions) || !targetVersion) { return null; } // ========== render ========== -if (isEmpty(targetOptions)) return null return (
🧹 Nitpick comments (2)
src/components/FileActions.tsx (2)
18-26: Reorder store helpers after store creation for clarity.The
setStoreandrestoreStorefunctions referenceuseFileActionsStorebefore it's defined (line 24). While this works due to function hoisting in JavaScript, it reduces readability and may confuse maintainers.🔎 Suggested reordering
-const setStore = (state: Partial<IState>) => - useFileActionsStore.setState(state, false, 'FileActions/setStore') - -const restoreStore = () => - useFileActionsStore.setState(initialState, false, 'FileActions/restoreStore') - const useFileActionsStore = create<IState>()( devtools(() => initialState, { name: 'FileActions' }) ); + +const setStore = (state: Partial<IState>) => + useFileActionsStore.setState(state, false, 'FileActions/setStore') + +const restoreStore = () => + useFileActionsStore.setState(initialState, false, 'FileActions/restoreStore')
41-52: Consider using URL parsing for safer filename extraction.The download handler uses
.split('/').pop()to extract the filename (line 47). While this works for typical URLs and has a fallback, using the URL API would be more robust for edge cases like URLs with query parameters or fragments.🔎 Alternative implementation
function handleDownloadRaw() { if (!state.fileContent) return; const blob = new Blob([state.fileContent], { type: 'text/plain' }); const url = window.URL.createObjectURL(blob); const link = document.createElement('a'); link.href = url; - link.download = state.rawUrl.split('/').pop() || 'file'; + try { + const urlObj = new URL(state.rawUrl); + link.download = urlObj.pathname.split('/').pop() || 'file'; + } catch { + link.download = state.rawUrl.split('/').pop() || 'file'; + } document.body.appendChild(link); link.click(); document.body.removeChild(link); window.URL.revokeObjectURL(url); }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package.json(1 hunks)src/components/CodeViewer.tsx(4 hunks)src/components/CustomTabs.tsx(2 hunks)src/components/FileActions.tsx(1 hunks)src/components/NPMVersionSelect.tsx(2 hunks)src/hooks/useFile.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (3)
src/hooks/useFile.ts (1)
src/config.ts (1)
REGISTRY(1-1)
src/components/CodeViewer.tsx (2)
src/hooks/useFile.ts (1)
useFileContent(65-73)src/components/FileActions.tsx (1)
FileActions(75-78)
src/components/CustomTabs.tsx (2)
src/components/FileActions.tsx (1)
FileActions(75-78)src/components/NPMVersionSelect.tsx (1)
NPMVersionSelect(89-171)
🪛 GitHub Actions: CI
src/components/FileActions.tsx
[error] 33-33: React Hook "useFileActionsStore" is called in function "_FileActions" that is neither a React function component nor a custom React Hook function. React component names must start with an uppercase letter. React Hook names must start with the word "use".
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (6)
src/components/FileActions.tsx (1)
60-72: LGTM! Clean guard and render logic.The early return when store values are empty (line 60) prevents rendering incomplete UI. The action buttons with tooltips provide good UX for copying, downloading, and viewing raw files.
src/components/CodeViewer.tsx (1)
73-73: LGTM! Correct usage of updated useFileContent API.The destructuring correctly handles the new tuple return type from
useFileContent, extracting both the code content and fileUri.src/components/CustomTabs.tsx (1)
48-64: LGTM! Clean integration of FileActions.The conditional rendering of FileActions (line 49) correctly limits it to the "files" tab, and the Flex layout provides a clean horizontal arrangement with NPMVersionSelect. The implementation aligns well with the PR objectives.
src/components/NPMVersionSelect.tsx (1)
143-170: LGTM! Clean layout refactor.The change from Space wrapper to fragment is appropriate since the layout responsibility has moved to the parent component (CustomTabs uses Flex). The rendering logic remains functionally equivalent.
package.json (1)
37-37: No changes needed. Zustand 5.0.9 is the latest version and was published 19 days ago. The dependency is current with no known security advisories.src/hooks/useFile.ts (1)
66-72: The refactor is properly implemented with all call sites updated.The separation of the SWR response and
fileUriinto a clean tuple is an idiomatic pattern that provides consumers access to the raw file URL. The single call site inCodeViewer.tsxcorrectly destructures the tuple:const [{ data: code }, { fileUri }] = useFileContent(...), confirming the breaking API change has been properly handled throughout the codebase.
Wxh16144
force-pushed
the
wuxh/file-actions
branch
from
ecde016 to
05cc153
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/NPMVersionSelect.tsx (1)
136-141: Remove duplicate early return check.Lines 136-138 already check
isEmpty(targetOptions)and returnnullif true. The additional check at line 141 is unreachable dead code and should be removed.🔎 Proposed fix
if (isEmpty(targetOptions) || !targetVersion) { return null; } - // ========== render ========== - if (isEmpty(targetOptions)) return null - return (
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
package.json(1 hunks)src/components/CodeViewer.tsx(4 hunks)src/components/CustomTabs.tsx(2 hunks)src/components/FileActions.tsx(1 hunks)src/components/NPMVersionSelect.tsx(2 hunks)src/hooks/useFile.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- src/components/FileActions.tsx
- src/components/CodeViewer.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
src/hooks/useFile.ts (1)
src/config.ts (1)
REGISTRY(1-1)
src/components/CustomTabs.tsx (2)
src/components/FileActions.tsx (1)
FileActions(76-79)src/components/NPMVersionSelect.tsx (1)
NPMVersionSelect(89-171)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Node.js / Test (ubuntu-latest, 24)
- GitHub Check: Node.js / Test (ubuntu-latest, 22)
🔇 Additional comments (4)
src/components/NPMVersionSelect.tsx (1)
143-170: Refactoring looks good.The change from Space wrapper to a React fragment is appropriate since the parent component (CustomTabs.tsx) now handles the layout with a Flex container.
src/components/CustomTabs.tsx (1)
48-64: Layout integration looks good.The Flex wrapper with appropriate gap and conditional rendering of FileActions when
activateKey === 'files'aligns well with the PR objectives to add file action buttons.package.json (1)
37-37: Zustand package version is valid and secure.Version ^5.0.9 is the current latest stable release with no known vulnerabilities.
src/hooks/useFile.ts (1)
65-72: All call sites properly updated for the tuple return type.The function signature change is correctly implemented in the single call site found at
src/components/CodeViewer.tsx:73, where the destructuring patternconst [{ data: code }, { fileUri }] = useFileContent(...)properly handles the new tuple return type[SWRResponse, { fileUri }].
| @@ -1,9 +1,10 @@ | |||
| 'use client'; | |||
| import { Tabs } from 'antd'; | |||
| import { Flex, Space, Tabs } from 'antd'; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if Space from antd is used in CustomTabs.tsx or NPMVersionSelect.tsx
echo "=== Checking Space usage in CustomTabs.tsx ==="
rg -n "Space" src/components/CustomTabs.tsx
echo -e "\n=== Checking Space usage in NPMVersionSelect.tsx ==="
rg -n "Space" src/components/NPMVersionSelect.tsxRepository: cnpm/cnpmweb
Length of output: 290
🏁 Script executed:
#!/bin/bash
# View CustomTabs.tsx to see if Space or NPMVersionSelect is used in the component
echo "=== CustomTabs.tsx full content ==="
cat -n src/components/CustomTabs.tsx
echo -e "\n=== Check for JSX usage of Space or NPMVersionSelect ==="
rg -n "<Space|<NPMVersionSelect" src/components/CustomTabs.tsxRepository: cnpm/cnpmweb
Length of output: 2852
Remove unused Space import from line 2.
The Space component from 'antd' is imported in CustomTabs.tsx but never used in the component. While NPMVersionSelect.tsx does use Space internally, each component should manage its own imports—parent components don't need to import what child components use.
import { Flex, Space, Tabs } from 'antd';
Change to:
import { Flex, Tabs } from 'antd';
🤖 Prompt for AI Agents
In src/components/CustomTabs.tsx around line 2, the import statement includes an
unused Space import from 'antd'; remove Space from the import list so it only
imports the symbols actually used (e.g., Flex and Tabs) to eliminate the unused
import and satisfy linting.
背景 or 需求
在 npmmirror 已经开启包白名单的前提下，我理解能在 npmmirror web 的产物预览看到的文件都默许开发者以 cdn 的方式调用。就像这样：
但是要得到这个地址是比较繁琐的：
min.cssindex.js所以，参考 GitHub 的文件页，将这个功能搬到 npmmirror web
效果截图
实现细节
因为 header 中组件需要得到文件信息，但是当前代码实现没有统一一个 global store。 所以临时用 zustand 方式，存储在内存中。并且最终通过
<CodeViewer />注入这些信息 （CodeViewer 代表用户当前查看到文件，看起来写的比较乱，但是这个可以最小化实现。另外我在实现的时候发现当前的 dark / light 主题等操作是有点问题的。所以后面再开一个新的 PR 修复。就不和当前放在一起了
Summary by CodeRabbit
New Features
Improvements
Chores
✏️ Tip: You can customize this high-level summary in your review settings.