-
-
Notifications
You must be signed in to change notification settings - Fork 211
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
Fix: Sidebar position sticky #1086
base: main
Are you sure you want to change the base?
Conversation
built with Refined Cloudflare Pages Action⚡ Cloudflare Pages Deployment
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1086 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 10 10
Lines 373 373
Branches 94 94
=========================================
Hits 373 373 ☔ View full report in Codecov by Sentry. |
@techmannih the sidebar should be sticky to such high extend. |
@DhairyaMajmudar I’m unable to understand your point. Please explain. I think it's sticky also |
I meant the sidebar should not be sticky to upto |
@DhairyaMajmudar Could you please clarify what you need? I’m having some difficulty understanding. |
@techmannih I was worried about the less bottom margin for the sidebar cause of which it was getting patched up with need help section you can check the video below Screencast.from.2024-11-10.12-07-28.webmThe expected behavior should be like this : ) Screencast.from.2024-11-10.12-10-15.webm |
Thanks @DhairyaMajmudar, it's a great clarification! Now I get your point clearly. please review now |
@DhairyaMajmudar @benjagm take a look this PR |
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.
@techmannih PR is working correct in local, but in hosted preview it's not showing expected behaviour. You need to merge the latest changes form main to your branch to fix this problem.
@DhairyaMajmudar done |
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.
LGTM 💯
Good work @techmannih |
@benjagm review this PR |
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.
The add component below the sidebar appears now cut missing almost 50% of the content. Can you please check.
@benjagm i think which is in scroll you can see on scrolling |
@benjagm take a look please 20250107143902.mp4 |
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.
There are still problems with this version when scrolling the sidebar with short pages. Please see the video how the sidebar appears confusingly cut and requires the user to scroll down on it. Expectation is to show all the content of the sidebar without having to scroll it.
See the recording demoing the issue here: https://drive.google.com/file/d/1vj5eCS2oym1Kope9bP8oqSwS6AeXFunI/view?usp=sharing
@benjagm yeah that was issue, now updated 20250111102058.mp4 |
What kind of change does this PR introduce?
This PR introduces a sticky sidebar position, allowing the sidebar to remain fixed in view while the user scrolls down the page.
Issue Number:
Issue #774
Closes #774
Screenshots/videos:
20241030144432.mp4