-
Notifications
You must be signed in to change notification settings - Fork 14
feat: outline position #115
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
WalkthroughA sticky "edit on GitHub" feature was added to the documentation layout. The component's DOM structure was reorganized with a new wrapper hierarchy containing a mask and content container, while CSS styling rules were introduced to position it as a sticky element above the outline with appropriate spacing and background handling. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@styles/doc-layout.css`:
- Around line 101-104: The negative margin declared on .edit-on-github is being
nullified by a later rule that sets margin-top: 0 !important; locate the
conflicting rule that targets .edit-on-github with margin-top: 0 !important and
either remove that override or confine it inside the mobile breakpoint media
query so the negative offset remains for larger viewports; if you must keep the
override, drop !important and use a scoped selector (e.g., within a mobile-only
wrapper or media query) so .edit-on-github retains its intended negative margin
outside mobile.
In `@theme/components/EditOnGithub/index.tsx`:
- Around line 55-64: Add rel="noopener noreferrer" to the external anchor
elements in the EditOnGithub component: update the anchor with href={editLink}
(containing <EditIcon> and t("edit_page")) and the anchor with href={issueLink}
(containing <GithubIcon> and t("create_issue")) to include rel="noopener
noreferrer" alongside target="_blank" to prevent tabnabbing.
🧹 Nitpick comments (1)
styles/doc-layout.css (1)
231-239: Consider replacing the hard-coded 59px sticky offset with a CSS variable.This offset likely depends on the edit-on-github block height; a variable would avoid drift if spacing or content changes.
♻️ Example approach
:root { + --rp-edit-on-github-height: 59px; } .rp-outline__title { - top: calc(var(--rp-content-padding-y) + 59px); + top: calc(var(--rp-content-padding-y) + var(--rp-edit-on-github-height)); }
| .edit-on-github { | ||
| margin-top: calc(-1 * var(--rp-content-padding-y)); | ||
| background: var(--rp-c-bg); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
cat -n styles/doc-layout.css | sed -n '95,110p'Repository: alauda/knowledge
Length of output: 479
🏁 Script executed:
cat -n styles/doc-layout.css | sed -n '215,235p'Repository: alauda/knowledge
Length of output: 608
🏁 Script executed:
cat -n styles/doc-layout.css | sed -n '200,230p'Repository: alauda/knowledge
Length of output: 1102
🏁 Script executed:
cat -n styles/doc-layout.css | sed -n '190,200p'Repository: alauda/knowledge
Length of output: 313
Resolve conflicting .edit-on-github margin-top rules.
The margin-top: 0 !important at line 223-225 overrides the negative margin defined at line 102 globally. This nullifies the intended offset behavior. Clarify whether the override is needed and either remove it or scope it to the mobile breakpoint where it applies.
🤖 Prompt for AI Agents
In `@styles/doc-layout.css` around lines 101 - 104, The negative margin declared
on .edit-on-github is being nullified by a later rule that sets margin-top: 0
!important; locate the conflicting rule that targets .edit-on-github with
margin-top: 0 !important and either remove that override or confine it inside
the mobile breakpoint media query so the negative offset remains for larger
viewports; if you must keep the override, drop !important and use a scoped
selector (e.g., within a mobile-only wrapper or media query) so .edit-on-github
retains its intended negative margin outside mobile.
| <a href={editLink} target="_blank" className="flex"> | ||
| <EditIcon></EditIcon> | ||
| <span className="ml-2">{t("edit_page")}</span> | ||
| </a> | ||
| </div> | ||
| <div className="editLink"> | ||
| <a href={issueLink} target="_blank" className="flex"> | ||
| <GithubIcon></GithubIcon> | ||
| <span className="ml-2">{t("create_issue")}</span> | ||
| </a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Add rel="noopener noreferrer" to external target="_blank" links.
target="_blank" without rel enables tabnabbing. Please add the rel attribute to both anchors.
🔒 Proposed fix
- <a href={editLink} target="_blank" className="flex">
+ <a href={editLink} target="_blank" rel="noopener noreferrer" className="flex">
<EditIcon></EditIcon>
<span className="ml-2">{t("edit_page")}</span>
</a>
...
- <a href={issueLink} target="_blank" className="flex">
+ <a href={issueLink} target="_blank" rel="noopener noreferrer" className="flex">
<GithubIcon></GithubIcon>
<span className="ml-2">{t("create_issue")}</span>
</a>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <a href={editLink} target="_blank" className="flex"> | |
| <EditIcon></EditIcon> | |
| <span className="ml-2">{t("edit_page")}</span> | |
| </a> | |
| </div> | |
| <div className="editLink"> | |
| <a href={issueLink} target="_blank" className="flex"> | |
| <GithubIcon></GithubIcon> | |
| <span className="ml-2">{t("create_issue")}</span> | |
| </a> | |
| <a href={editLink} target="_blank" rel="noopener noreferrer" className="flex"> | |
| <EditIcon></EditIcon> | |
| <span className="ml-2">{t("edit_page")}</span> | |
| </a> | |
| </div> | |
| <div className="editLink"> | |
| <a href={issueLink} target="_blank" rel="noopener noreferrer" className="flex"> | |
| <GithubIcon></GithubIcon> | |
| <span className="ml-2">{t("create_issue")}</span> | |
| </a> |
🤖 Prompt for AI Agents
In `@theme/components/EditOnGithub/index.tsx` around lines 55 - 64, Add
rel="noopener noreferrer" to the external anchor elements in the EditOnGithub
component: update the anchor with href={editLink} (containing <EditIcon> and
t("edit_page")) and the anchor with href={issueLink} (containing <GithubIcon>
and t("create_issue")) to include rel="noopener noreferrer" alongside
target="_blank" to prevent tabnabbing.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.