Fixed heading anchor icon misalignment and heading formatting for Kanvas documentation section - #1197
Conversation
📝 WalkthroughWalkthroughThe heading anchor layout now uses inline-block positioning and non-wrapping content. The “Publishing from Kanvas” heading no longer includes the trailing link emoji. ChangesHeading presentation updates
Estimated code review effort: 1 (Trivial) | ~2 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@assets/scss/_elements_project.scss`:
- Line 167: Update the .heading-link styles by removing the flex-only
declarations, then set .heading-anchor to display: inline-block while preserving
its existing transform and nowrap behavior. Ensure the heading text and anchor
use inline flow so the anchor remains immediately after the final word when
headings wrap.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c0ace054-1fde-4f8c-bb2b-f1131afdc818
📒 Files selected for processing (1)
assets/scss/_elements_project.scss
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
assets/scss/_elements_project.scss (2)
158-172: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winAdd a visible keyboard-focus state for the anchor.
The template renders
.heading-anchoras an<a>with anaria-label. This rule sets the anchor toopacity: 0and reveals it only on hover. Keyboard users can focus the link without hover and see no visible anchor or focus state. Add a:focus-visibleor:focus-withinrule that restores visibility.Proposed fix
&:hover .heading-anchor { opacity: 1; transform: translateY(0); } + &:focus-within .heading-anchor, + .heading-anchor:focus-visible { + opacity: 1; + transform: translateY(0); + } }As per coding guidelines, interactive elements must support keyboard navigation and use ARIA labels when semantic HTML is insufficient.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/scss/_elements_project.scss` around lines 158 - 172, Update the `.heading-anchor` styles to add a visible keyboard-focus state using `:focus-visible` or `:focus-within`, restoring opacity (and the intended visible transform) when the anchor receives keyboard focus while preserving the existing hover behavior.Source: Coding guidelines
154-173: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winRestore the
.heading-linknesting before merging.Line 156 closes
.heading-linkbefore.heading-anchorand&:hover .heading-anchor. The&selector at Line 169 has no parent selector, so Sass compilation fails. Move the closing brace to after the hover rule.Proposed fix
.heading-link { position: relative; -} - - .heading-anchor { + .heading-anchor { display: inline-block; font-size: 0.6em; opacity: 0; transform: translateY(2px); transition: opacity 0.3s ease, transform 0.3s ease; margin-left: 0.25rem; text-decoration: none; white-space: nowrap; } &:hover .heading-anchor { opacity: 1; transform: translateY(0); } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@assets/scss/_elements_project.scss` around lines 154 - 173, Restore the `.heading-link` nesting in the `.heading-link` rule by moving its closing brace after the nested `.heading-anchor` and `&:hover .heading-anchor` rules, ensuring the parent selector remains valid for Sass compilation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@assets/scss/_elements_project.scss`:
- Around line 158-172: Update the `.heading-anchor` styles to add a visible
keyboard-focus state using `:focus-visible` or `:focus-within`, restoring
opacity (and the intended visible transform) when the anchor receives keyboard
focus while preserving the existing hover behavior.
- Around line 154-173: Restore the `.heading-link` nesting in the
`.heading-link` rule by moving its closing brace after the nested
`.heading-anchor` and `&:hover .heading-anchor` rules, ensuring the parent
selector remains valid for Sass compilation.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 9b8c6718-273c-4205-ab33-74a60e9735b0
📒 Files selected for processing (1)
assets/scss/_elements_project.scss
dhruveshmishra
left a comment
dhruveshmishra
left a comment
There was a problem hiding this comment.
@RaIyAn90 Why have u created 2 issues for a same issue of same repo, u can cover it for the whole docs by same pr. Also make sure do not raise the pr directly, wait for someone to get u assigned to that issue, and just not resolve the suggestions given by the ai address them properly if neccesary then do the changes if not then give proper reason.
Hi @dhruveshmishra, thanks for the feedback. I split them because they seemed to have different root causes one is a CSS layout bug and the other resolve by changing in .md file. I thought separating would make review cleaner. should I close #1199 and cover both in the existing issue and PR, or would you prefer I close the current PR and reopen once both issues are consolidated and I'm assigned? On the PR being raised before assignment apologies, I should have waited. Noted for future contributions. On the AI-review comments, I understood. I'll re-read the suggestions carefully, respond to each with reasoning, and only push changes where the suggestion is actually improving the fix. Will update the PR shortly. |
|
If u think it coorect then u can close the other issue and add it on this issue only by making a secondary point, make a commit in this pr only for the other changes. |
|
@RaIyAn90 As long as the fixes are closely related and not significantly different in scope, keeping them in one issue and PR makes it easier for the reviewer to understand the overall change and review everything together. |
RaIyAn90
changed the title
|
@dhruveshmishra, @Utkarsh-Mauryaa I've made the changes and merged the two issues into one as well the PRs. Now you can check this out. |
RaIyAn90
changed the title
banana-three-join
left a comment
banana-three-join
left a comment
There was a problem hiding this comment.
For a change this small, 4 commits aren't needed, so please, squash your commits. Also, for reviews, please provide a response to all of the given suggestions to determine whether the suggestion itself is valid or not.
3942366 to
cd78ac3
Compare
Signed-off-by: Mubassir Raiyan <111300457+RaIyAn90@users.noreply.github.com> Signed-off-by: RaIyAn90 <samin.raiyan09@gmail.com>
|
Thank you for contributing to the Layer5 community! 🎉 \ \ |
4 participants
Notes for Reviewers
This PR fixes #1196.
2026-08-09.21-49-44.mp4
Signed commits
Summary by CodeRabbit
Summary by CodeRabbit