Skip to content

[Sidebar] Resolve sidebar moving on scroll#965

Open
Bharath314 wants to merge 2 commits intolayer5io:masterfrom
Bharath314:sidebar
Open

[Sidebar] Resolve sidebar moving on scroll#965
Bharath314 wants to merge 2 commits intolayer5io:masterfrom
Bharath314:sidebar

Conversation

@Bharath314
Copy link
Copy Markdown
Contributor

Notes for Reviewers

This PR fixes #468.

2026-04-28.13-23-50.mp4

Signed commits

  • Yes, I signed my commits.

Signed-off-by: Bharath Valaboju <69413757+Bharath314@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Apr 28, 2026

🚀 Preview deployment: https://layer5io.github.io/docs/pr-preview/pr-965/

Note: Preview may take a moment (GitHub Pages deployment in progress). Please wait and refresh. Track deployment here

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request modifies the sidebar styling in assets/scss/_styles_project.scss by adding a new inner element and refining media queries for sticky positioning. The review feedback identifies several redundant CSS properties, such as default values for order and position, and unnecessary overflow rules that should be removed to simplify the styles.

Comment thread assets/scss/_styles_project.scss Outdated
Comment on lines +240 to +248
order: 0;

@include media-breakpoint-up(md) {
@supports (position: sticky) {
position: static;
top: 0;
z-index: 10;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The properties in this block are redundant or ineffective:

  • order: 0 is the default value for flex items.
  • position: static is the default position, and top: 0 is ignored on static elements.
  • z-index: 10 on a static element does not affect stacking relative to elements outside the parent container.

If the goal is to ensure the sidebar stays on top of other page elements (like the footer which has z-index: 5), consider applying z-index to the sticky .td-sidebar container instead.

Comment thread assets/scss/_styles_project.scss Outdated
Comment on lines +269 to +278
$_max-height: none;
overflow: hidden;
padding: 0px;

@include media-breakpoint-up(md) {
@supports (position: sticky) {
max-height: $_max-height;
overflow-y: hidden;
}
}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This block introduces redundant logic. max-height: none is the default value, and overflow-y: hidden is already applied via overflow: hidden on line 270. This can be simplified by removing the variable and the media query.

  overflow: hidden;
  padding: 0px;

Signed-off-by: Bharath Valaboju <69413757+Bharath314@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

left sidebar not static while scrolling up the page

1 participant