fix(dashboard): restore README dialog anchor navigation#6083
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 an issue where anchor navigation within the README dialog was not functioning correctly. It introduces robust heading ID generation, intercepts internal anchor clicks to enable smooth scrolling to the target sections, and improves the user experience by adding a scroll margin to ensure headings are fully visible when navigated to. Highlights
Changelog
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 个问题,并给出了一些高层次的反馈:
- 在直接使用
CSS.escape的情况下,如果运行环境中没有定义它,会抛出异常;建议在调用querySelector之前先做特性检测,并提供一个简单的回退逻辑(例如:const safeId = CSS?.escape ? CSS.escape(targetId) : targetId;)。 decodeURIComponent(rawHref.slice(1))在遇到格式错误的编码时会抛出异常;相比之下,把它包在 try/catch 中,在锚点值非法时忽略或提前退出,可能会比让点击处理函数报错更安全。
给 AI Agent 的提示
Please address the comments from this code review:
## Overall Comments
- Using `CSS.escape` directly can throw in environments where it's not defined; consider guarding with a feature check and a simple fallback (e.g. `const safeId = CSS?.escape ? CSS.escape(targetId) : targetId;`) before calling `querySelector`.
- `decodeURIComponent(rawHref.slice(1))` will throw on malformed encodings; it may be safer to wrap this in a try/catch and ignore/bail out on invalid anchor values instead of letting the click handler error.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="302-303" />
<code_context>
+ const anchor = event.target.closest('a[href^="#"]');
+ if (!anchor) return;
+
+ const rawHref = anchor.getAttribute("href");
+ const targetId = rawHref ? decodeURIComponent(rawHref.slice(1)) : "";
+ if (!targetId) return;
+
</code_context>
<issue_to_address>
**issue (bug_risk):** decodeURIComponent on the fragment can throw on malformed encodings, which would break the click handler.
This assumes the fragment is always valid percent-encoding; an invalid `%` sequence will cause `decodeURIComponent` to throw and stop the handler. Consider guarding with a try/catch and falling back to the raw substring:
```js
let targetId = "";
if (rawHref) {
try {
targetId = decodeURIComponent(rawHref.slice(1));
} catch {
targetId = rawHref.slice(1);
}
}
```
</issue_to_address>帮我变得更有用!请在每条评论上点 👍 或 👎,我会根据你的反馈改进后续的评审。
Original comment in English
Hey - I've found 1 issue, and left some high level feedback:
- Using
CSS.escapedirectly can throw in environments where it's not defined; consider guarding with a feature check and a simple fallback (e.g.const safeId = CSS?.escape ? CSS.escape(targetId) : targetId;) before callingquerySelector. decodeURIComponent(rawHref.slice(1))will throw on malformed encodings; it may be safer to wrap this in a try/catch and ignore/bail out on invalid anchor values instead of letting the click handler error.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- Using `CSS.escape` directly can throw in environments where it's not defined; consider guarding with a feature check and a simple fallback (e.g. `const safeId = CSS?.escape ? CSS.escape(targetId) : targetId;`) before calling `querySelector`.
- `decodeURIComponent(rawHref.slice(1))` will throw on malformed encodings; it may be safer to wrap this in a try/catch and ignore/bail out on invalid anchor values instead of letting the click handler error.
## Individual Comments
### Comment 1
<location path="dashboard/src/components/shared/ReadmeDialog.vue" line_range="302-303" />
<code_context>
+ const anchor = event.target.closest('a[href^="#"]');
+ if (!anchor) return;
+
+ const rawHref = anchor.getAttribute("href");
+ const targetId = rawHref ? decodeURIComponent(rawHref.slice(1)) : "";
+ if (!targetId) return;
+
</code_context>
<issue_to_address>
**issue (bug_risk):** decodeURIComponent on the fragment can throw on malformed encodings, which would break the click handler.
This assumes the fragment is always valid percent-encoding; an invalid `%` sequence will cause `decodeURIComponent` to throw and stop the handler. Consider guarding with a try/catch and falling back to the raw substring:
```js
let targetId = "";
if (rawHref) {
try {
targetId = decodeURIComponent(rawHref.slice(1));
} catch {
targetId = rawHref.slice(1);
}
}
```
</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 rawHref = anchor.getAttribute("href"); | ||
| const targetId = rawHref ? decodeURIComponent(rawHref.slice(1)) : ""; |
There was a problem hiding this comment.
issue (bug_risk): 在片段(fragment)上调用 decodeURIComponent 时,如果遇到格式错误的编码,会抛出异常,从而导致点击处理函数出错。
这里假设片段总是合法的百分号编码;但一旦出现无效的 % 序列,就会让 decodeURIComponent 抛出异常并中止处理函数。建议用 try/catch 做保护,并在出错时回退到原始子串:
let targetId = "";
if (rawHref) {
try {
targetId = decodeURIComponent(rawHref.slice(1));
} catch {
targetId = rawHref.slice(1);
}
}Original comment in English
issue (bug_risk): decodeURIComponent on the fragment can throw on malformed encodings, which would break the click handler.
This assumes the fragment is always valid percent-encoding; an invalid % sequence will cause decodeURIComponent to throw and stop the handler. Consider guarding with a try/catch and falling back to the raw substring:
let targetId = "";
if (rawHref) {
try {
targetId = decodeURIComponent(rawHref.slice(1));
} catch {
targetId = rawHref.slice(1);
}
}There was a problem hiding this comment.
Code Review
This pull request effectively restores anchor navigation within the README dialog. It introduces a slugifyHeading function to generate stable IDs for headings, intercepts clicks on anchor links to provide smooth scrolling, and adds a top margin for better visibility of the scrolled-to section. The implementation is solid. I've only suggested a minor improvement to the slug generation logic to make it more robust by trimming leading/trailing hyphens, which is a common practice for creating clean slugs.
| const base = (text || "") | ||
| .trim() | ||
| .toLowerCase() | ||
| .normalize("NFKD") | ||
| .replace(/[\u0300-\u036f]/g, "") | ||
| .replace(/[^\p{Letter}\p{Number}\s-]/gu, "") | ||
| .replace(/\s+/g, "-") | ||
| .replace(/-+/g, "-"); |
There was a problem hiding this comment.
The current slug generation logic can produce slugs with leading or trailing hyphens (e.g., from a heading like ## - My Title -). While these are valid in HTML5 IDs, it's common practice to trim them for cleaner and more predictable URLs/anchors. I suggest adding a step to remove any leading or trailing hyphens from the generated slug.
const base = (text || "")
.trim()
.toLowerCase()
.normalize("NFKD")
.replace(/[\u0300-\u036f]/g, "")
.replace(/[^\p{Letter}\p{Number}\s-]/gu, "")
.replace(/\s+/g, "-")
.replace(/-+/g, "-")
.replace(/^-+|-+$/g, "");
Summary
ReadmeDialog#anchorclicks and smoothly scroll the dialog container to the matching sectionVerification
corepack pnpm run typecheckcorepack pnpm run buildCloses #5914.
Summary by Sourcery
在 README 对话框中恢复对话框内锚点导航和标题定位行为。
Bug Fixes:
Enhancements:
Original summary in English
Summary by Sourcery
Restore in-dialog anchor navigation and heading targeting behavior in the README dialog.
Bug Fixes:
Enhancements: