fix(dashboard): make plugin readme toc anchors work in dialog#5915
fix(dashboard): make plugin readme toc anchors work in dialog#5915Clhikari wants to merge 3 commits intoAstrBotDevs:masterfrom
Conversation
Summary of ChangesHello, 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! This pull request addresses a critical usability issue in the dashboard where anchor links within plugin README dialogs were not working correctly. The changes ensure that users can now navigate seamlessly through the documentation by clicking on table of contents entries. This was achieved by standardizing the generation of heading IDs and implementing custom scrolling behavior for internal links within the dialog, significantly improving the user experience for plugin documentation. Highlights
Changelog
Activity
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
|
There was a problem hiding this comment.
Hey - 我发现了 1 个问题,并给出了一些高层次的反馈:
heading_open渲染规则会在每次renderedHtml重新计算时被重新创建;建议在初始化md时只注册一次该规则(或防止重复赋值),以避免重复的初始化操作,并避免将来其他代码自定义标题渲染时产生潜在冲突。- 在
scrollToHeadingInDialog中,依赖closest('.v-card-text')会将滚动行为与特定的 DOM 结构强耦合;使用一个专门的 ref 或更明确的容器元素作为滚动目标,可以让对话框在布局发生变化时更加稳健。
供 AI Agent 使用的提示
Please address the comments from this code review:
## Overall Comments
- The `heading_open` renderer rule is recreated on every `renderedHtml` recomputation; consider registering this rule once when `md` is initialized (or guarding against multiple assignments) to avoid redundant setup and potential conflicts if other code later customizes heading rendering.
- In `scrollToHeadingInDialog`, relying on `closest('.v-card-text')` couples the scroll behavior to a specific DOM structure; using a dedicated ref or a more explicit container element for the scroll target would make the dialog behavior more robust to layout changes.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="36-38" />
<code_context>
+ return href?.startsWith("#") && href.length > 1 && !href.startsWith("#/");
+}
+
+function scrollToHeadingInDialog(anchorId, markdownContainer) {
+ if (!anchorId || !markdownContainer) return;
+ const decodedId = decodeURIComponent(anchorId);
+ const escapedId =
+ typeof CSS !== "undefined" && CSS.escape
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard `decodeURIComponent` to avoid runtime errors on malformed anchors.
If `anchorId` has invalid `%` encoding (e.g., from malformed links), `decodeURIComponent` will throw and break the handler. Consider wrapping it in a try/catch and falling back to the original value:
```js
let decodedId;
try {
decodedId = decodeURIComponent(anchorId);
} catch {
decodedId = anchorId;
}
```
This keeps the dialog working even with bad anchors.
</issue_to_address>帮我变得更有用!请对每条评论点 👍 或 👎,我会根据你的反馈改进之后的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- The
heading_openrenderer rule is recreated on everyrenderedHtmlrecomputation; consider registering this rule once whenmdis initialized (or guarding against multiple assignments) to avoid redundant setup and potential conflicts if other code later customizes heading rendering. - In
scrollToHeadingInDialog, relying onclosest('.v-card-text')couples the scroll behavior to a specific DOM structure; using a dedicated ref or a more explicit container element for the scroll target would make the dialog behavior more robust to layout changes.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `heading_open` renderer rule is recreated on every `renderedHtml` recomputation; consider registering this rule once when `md` is initialized (or guarding against multiple assignments) to avoid redundant setup and potential conflicts if other code later customizes heading rendering.
- In `scrollToHeadingInDialog`, relying on `closest('.v-card-text')` couples the scroll behavior to a specific DOM structure; using a dedicated ref or a more explicit container element for the scroll target would make the dialog behavior more robust to layout changes.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="36-38" />
<code_context>
+ return href?.startsWith("#") && href.length > 1 && !href.startsWith("#/");
+}
+
+function scrollToHeadingInDialog(anchorId, markdownContainer) {
+ if (!anchorId || !markdownContainer) return;
+ const decodedId = decodeURIComponent(anchorId);
+ const escapedId =
+ typeof CSS !== "undefined" && CSS.escape
</code_context>
<issue_to_address>
**issue (bug_risk):** Guard `decodeURIComponent` to avoid runtime errors on malformed anchors.
If `anchorId` has invalid `%` encoding (e.g., from malformed links), `decodeURIComponent` will throw and break the handler. Consider wrapping it in a try/catch and falling back to the original value:
```js
let decodedId;
try {
decodedId = decodeURIComponent(anchorId);
} catch {
decodedId = anchorId;
}
```
This keeps the dialog working even with bad anchors.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
There was a problem hiding this comment.
Code Review
本次 PR 修复了插件 README 弹窗中目录锚点无法跳转的问题,实现很好。核心改动包括:使用 github-slugger 保证锚点 ID 的稳定性,并增加了在弹窗内处理锚点点击和滚动的逻辑。
我提出了两点改进建议:
- 优化
GithubSlugger实例和markdown-it渲染规则的初始化时机,避免在computed属性中重复创建,以提升性能和代码可维护性。 - 增强
CSS.escape的回退实现,以兼容不支持此 API 的旧版浏览器,确保在所有环境下锚点都能正常工作。
整体来看,这是一个高质量的修复,解决了用户体验上的一个痛点。
| const slugger = new GithubSlugger(); | ||
|
|
||
| md.renderer.rules.heading_open = (tokens, idx, options, _env, self) => { | ||
| const headingToken = tokens[idx + 1]; | ||
| const headingText = headingToken?.content || ""; | ||
| const headingId = slugger.slug(headingText); | ||
| tokens[idx].attrSet("id", headingId || slugger.slug("section")); | ||
| return self.renderToken(tokens, idx, options); | ||
| }; |
There was a problem hiding this comment.
markdown-it 的渲染规则(md.renderer.rules.heading_open)和 GithubSlugger 实例在 computed 属性中被重复定义和创建。这不仅效率低下,而且不是 markdown-it 的正确使用方式。渲染规则应该在 markdown-it 实例初始化后定义一次。
建议进行如下重构:
- 将
const slugger = new GithubSlugger();和md.renderer.rules.heading_open = ...的定义移到<script setup>的顶层作用域,与md实例的初始化放在一起。 - 在
renderedHtmlcomputed 属性的开头,调用slugger.reset()来为每次新的渲染重置其内部状态。
这将提高性能并使代码更具可维护性。
| const escapedId = | ||
| typeof CSS !== "undefined" && CSS.escape | ||
| ? CSS.escape(decodedId) | ||
| : decodedId.replace(/["\\]/g, "\\$&"); |
There was a problem hiding this comment.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我发现了 1 个问题
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="48" />
<code_context>
+
+function scrollToHeadingInDialog(anchorId, markdownContainer, scrollContainer) {
+ if (!anchorId || !markdownContainer) return;
+ const decodedId = decodeURIComponent(anchorId);
+ const escapedId =
+ typeof CSS !== "undefined" && CSS.escape
</code_context>
<issue_to_address>
**suggestion (bug_risk):** 为 `decodeURIComponent` 增加针对不合法锚点值的防护,以避免运行时错误。
因为 href 来自任意的 README 内容,一个格式错误的百分号编码锚点(例如 `#foo%2`)可能会导致 `decodeURIComponent` 抛出异常,从而破坏对话框的点击处理逻辑。请将这个调用包裹在 try/catch 中,如果出错则要么跳过处理,要么回退到直接使用原始的 `anchorId`,以确保在遇到不良输入时仍能保持健壮的行为。
</issue_to_address>帮我变得更有用!请在每条评论上点击 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue
Prompt for AI Agents
Please address the comments from this code review:
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="48" />
<code_context>
+
+function scrollToHeadingInDialog(anchorId, markdownContainer, scrollContainer) {
+ if (!anchorId || !markdownContainer) return;
+ const decodedId = decodeURIComponent(anchorId);
+ const escapedId =
+ typeof CSS !== "undefined" && CSS.escape
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Guard `decodeURIComponent` against malformed anchor values to avoid runtime errors.
Because the href comes from arbitrary README content, a malformed percent-encoded anchor (e.g. `#foo%2`) would cause `decodeURIComponent` to throw and break the dialog click handler. Please wrap this call in try/catch and either skip handling on error or fall back to using the raw `anchorId` to keep behavior resilient to bad input.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
|
@sourcery-ai review |
There was a problem hiding this comment.
Hey - 我已经查看了你的更改,看起来很棒!
帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进之后的代码审查。
Original comment in English
Hey - I've reviewed your changes and they look great!
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Motivation / 动机
Fixes #5914
Plugin README/文档弹窗中的目录锚点点击无效,无法跳转到对应章节。
根因是:标题锚点生成不稳定 + Hash 路由场景下目录
#...点击未在弹窗内处理滚动。Modifications / 改动点
github-slugger生成 Markdown 标题锚点id,替换原先自定义正则 slug 逻辑。ReadmeDialog中处理文档内锚点点击:#...;#/...(避免干扰现有 hash 路由);_blank + noopener,noreferrer;核心变更文件:
dashboard/src/components/shared/ReadmeDialog.vuedashboard/package.jsondashboard/pnpm-lock.yamlThis is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
output.mp4
Checklist / 检查清单
requirements.txt和pyproject.toml文件相应位置。/ I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations inrequirements.txtandpyproject.toml.Summary by Sourcery
改进插件 README 对话框中的锚点处理,以在对话框内可靠地导航到 Markdown 标题。
Bug 修复:
增强功能:
构建:
github-slugger依赖,并相应更新锁定文件。Original summary in English
Summary by Sourcery
Improve plugin README dialog anchor handling for reliable in-dialog navigation of markdown headings.
Bug Fixes:
Enhancements:
Build:
Bug 修复:
增强功能:
构建:
github-slugger依赖,并相应更新锁定文件。Original summary in English
Summary by Sourcery
改进插件 README 对话框中的锚点处理,以在对话框内可靠地导航到 Markdown 标题。
Bug 修复:
增强功能:
构建:
github-slugger依赖,并相应更新锁定文件。Original summary in English
Summary by Sourcery
Improve plugin README dialog anchor handling for reliable in-dialog navigation of markdown headings.
Bug Fixes:
Enhancements:
Build:
Bug 修复:
增强功能:
构建:
github-slugger依赖添加到仪表盘前端,并相应更新锁定文件(lockfile)。Original summary in English
Summary by Sourcery
改进插件 README 对话框中的锚点处理,以在对话框内可靠地导航到 Markdown 标题。
Bug 修复:
增强功能:
构建:
github-slugger依赖,并相应更新锁定文件。Original summary in English
Summary by Sourcery
Improve plugin README dialog anchor handling for reliable in-dialog navigation of markdown headings.
Bug Fixes:
Enhancements:
Build:
Bug 修复:
增强功能:
构建:
github-slugger依赖,并相应更新锁定文件。Original summary in English
Summary by Sourcery
改进插件 README 对话框中的锚点处理,以在对话框内可靠地导航到 Markdown 标题。
Bug 修复:
增强功能:
构建:
github-slugger依赖,并相应更新锁定文件。Original summary in English
Summary by Sourcery
Improve plugin README dialog anchor handling for reliable in-dialog navigation of markdown headings.
Bug Fixes:
Enhancements:
Build: