feat: add PluginDetailPage component for detailed plugin information display#7896
feat: add PluginDetailPage component for detailed plugin information display#7896
Conversation
…display refactor: remove extension preference storage management and related tests chore: clean up useExtensionPage by removing unused preference storage logic
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In InstalledPluginsTab and ExtensionCard the navigation is wired via a click on the whole ExtensionCard, but the action buttons inside the card (configure, uninstall, etc.) don’t stop event propagation, so clicking them will also open the detail page; consider moving navigation to a dedicated control or adding
@click.stopon those action buttons. - This change removes several installed-plugins UX features (list view, status filter, show/hide system plugins, pinning, sort options, etc.); if those behaviours are still needed, it might be better to keep them and integrate the detail view rather than dropping them entirely.
- PluginDetailPage introduces a direct
axioscall for/api/plugin/readme; if you already have a shared API/client helper or error-handling pattern elsewhere in the dashboard, consider routing this request through that to keep networking concerns consistent.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In InstalledPluginsTab and ExtensionCard the navigation is wired via a click on the whole ExtensionCard, but the action buttons inside the card (configure, uninstall, etc.) don’t stop event propagation, so clicking them will also open the detail page; consider moving navigation to a dedicated control or adding `@click.stop` on those action buttons.
- This change removes several installed-plugins UX features (list view, status filter, show/hide system plugins, pinning, sort options, etc.); if those behaviours are still needed, it might be better to keep them and integrate the detail view rather than dropping them entirely.
- PluginDetailPage introduces a direct `axios` call for `/api/plugin/readme`; if you already have a shared API/client helper or error-handling pattern elsewhere in the dashboard, consider routing this request through that to keep networking concerns consistent.
## Individual Comments
### Comment 1
<location path="dashboard/src/views/extension/PluginDetailPage.vue" line_range="308-311" />
<code_context>
+ });
+};
+
+const openExternal = (url) => {
+ if (!url) return;
+ window.open(url, "_blank", "noopener,noreferrer");
+};
+
</code_context>
<issue_to_address>
**🚨 suggestion (security):** Validate external URLs before opening them in a new tab
Plugin metadata may be untrusted, and `openExternal` forwards any string to `window.open`. Even with `noopener,noreferrer`, this still allows `javascript:` or other non-HTTP(S) URLs. Please restrict `window.open` calls to an allowlist of safe protocols (e.g., only `http:`/`https:`).
```suggestion
const openExternal = (url) => {
if (!url) return;
let parsed: URL;
try {
// Support both absolute and relative URLs while enforcing protocol
parsed = new URL(url, window.location.href);
} catch {
// Invalid URL format
return;
}
const protocol = parsed.protocol.toLowerCase();
const allowedProtocols = new Set(["http:", "https:"]);
if (!allowedProtocols.has(protocol)) {
return;
}
window.open(parsed.toString(), "_blank", "noopener,noreferrer");
};
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| const openExternal = (url) => { | ||
| if (!url) return; | ||
| window.open(url, "_blank", "noopener,noreferrer"); | ||
| }; |
There was a problem hiding this comment.
🚨 suggestion (security): Validate external URLs before opening them in a new tab
Plugin metadata may be untrusted, and openExternal forwards any string to window.open. Even with noopener,noreferrer, this still allows javascript: or other non-HTTP(S) URLs. Please restrict window.open calls to an allowlist of safe protocols (e.g., only http:/https:).
| const openExternal = (url) => { | |
| if (!url) return; | |
| window.open(url, "_blank", "noopener,noreferrer"); | |
| }; | |
| const openExternal = (url) => { | |
| if (!url) return; | |
| let parsed: URL; | |
| try { | |
| // Support both absolute and relative URLs while enforcing protocol | |
| parsed = new URL(url, window.location.href); | |
| } catch { | |
| // Invalid URL format | |
| return; | |
| } | |
| const protocol = parsed.protocol.toLowerCase(); | |
| const allowedProtocols = new Set(["http:", "https:"]); | |
| if (!allowedProtocols.has(protocol)) { | |
| return; | |
| } | |
| window.open(parsed.toString(), "_blank", "noopener,noreferrer"); | |
| }; |
There was a problem hiding this comment.
Code Review
This pull request introduces a dedicated plugin detail page and refactors the extension management interface. Key changes include the removal of pinning, drag-and-drop, and list-view functionalities in favor of a more detailed view that displays plugin handlers and documentation. The extension card was also updated with a new layout and improved metadata display. Feedback for the new PluginDetailPage.vue component suggests refining the Markdown sanitization configuration to follow the principle of least privilege and removing redundant scroll event listeners on both window and document to optimize performance.
| const renderMarkdown = (source) => { | ||
| const normalizedSource = String(source || "") | ||
| .replace(/[“”]/g, '"') | ||
| .replace(/[‘’]/g, "'"); | ||
| const rawHtml = markdown.render(normalizedSource); | ||
| const cleanHtml = DOMPurify.sanitize(rawHtml, MARKDOWN_SANITIZE_OPTIONS); | ||
| const container = document.createElement("div"); | ||
| container.innerHTML = cleanHtml; | ||
|
|
||
| container.querySelectorAll("a").forEach((link) => { | ||
| const href = link.getAttribute("href") || ""; | ||
| if (href.startsWith("http") || href.startsWith("//")) { | ||
| link.setAttribute("target", "_blank"); | ||
| link.setAttribute("rel", "noopener noreferrer"); | ||
| } | ||
| }); | ||
|
|
||
| return container.innerHTML; |
There was a problem hiding this comment.
| onMounted(() => { | ||
| updateHeaderStuckState(); | ||
| window.addEventListener("scroll", updateHeaderStuckState, { passive: true }); | ||
| document.addEventListener("scroll", updateHeaderStuckState, { | ||
| capture: true, | ||
| passive: true, | ||
| }); | ||
| }); | ||
|
|
||
| onBeforeUnmount(() => { | ||
| window.removeEventListener("scroll", updateHeaderStuckState); | ||
| document.removeEventListener("scroll", updateHeaderStuckState, { | ||
| capture: true, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The scroll event listener is added to both window and document. This is redundant and can lead to performance issues. Attaching the listener to window is sufficient for tracking scroll position.
onMounted(() => { updateHeaderStuckState(); window.addEventListener('scroll', updateHeaderStuckState, { passive: true }); }); onBeforeUnmount(() => { window.removeEventListener('scroll', updateHeaderStuckState); });
refactor: remove extension preference storage management and related tests
chore: clean up useExtensionPage by removing unused preference storage logic
Modifications / 改动点
Screenshots or Test Results / 运行截图或测试结果
Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了“验证步骤”和“运行截图”。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Introduce a dedicated plugin detail view for installed extensions and simplify the installed plugins list UI by removing pinning, list-view, and preference-based filtering/sorting features.
New Features:
Enhancements:
Chores: