fix: auto scroll of sidebar on navigating to different section in docs #1668
Conversation
Priyansh61
requested review from
derberg,
mcturco,
akshatnema,
magicmatatjahu and
asyncapi-bot-eve
as code owners
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site settings. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-1668--asyncapi-website.netlify.app/ |
Priyansh61
changed the title
|
Do mention in the issue before creating a pr for the issue! |
Priyansh61
changed the title
Done. Thanks @Savio629 |
|
/autoupdate |
There was a problem hiding this comment.
@Priyansh61 Thanks for creating PR in order to resolve the issue. I have a small concern, whenever I click on any section of docs inside Navbar, it scrolls the sidebar to that section but also it slightly scroll down the respective page in certain aspect. Kindly fix it.
|
Hi @akshatnema , Thanks for the review. Actually, while looking for this issue, there is a problem with the web Solution : I saw one more Another possible solution can be using the https://github.com/scroll-into-view/scroll-into-view-if-needed , this does the work and makes the site bullet proof and is compatible with all the browsers, but we need to import one more library in our codebase to accomplish this task. I tried manually writting the function, but its behaviour was very random the calculated offset worked perfectly in some cases, while in other it scrolled too much. Some help here would be really great. Thanks, |
|
Hey @magicmatatjahu, can you please look into the issue. Like should we import new library to target this issue or we can work with cdn also? Or you think that it isn't a big issue and we should go with present implementation inside PR? Your opinions needed... |
|
@akshatnema We can try to write own implementation. If it's difficult/time consuming we can go with library (not cdn). |
|
yo folks, what is the story with this one? I do not see a problem that we add new library to dependencies, we already have many 😄 |
The main story is that we are tackling a minute problem regarding scroll, for which it is required to add a dependency. And moreover, this dependency will not be used anywhere else to add any functionality. So, will adding a dependency be a good case to handle this problem? |
|
So I should then add the library and update the PR? |
|
@akshatnema ah not I get your motivation. In this case you are actually right as this fix will anyway be obsolete pretty soon because we critically need #1299 |
|
ok @derberg. Thanks for the update. But inside future sidebar as well, whenever we directly render a doc page using @Priyansh61 you can go with adding new dependency to resolve the error. |
|
@Priyansh61 any updates on this? |
|
Hey @Priyansh61, we are closing this PR as of no response from you. Kindly make a separate PR if you continue to work on this. |
Description
The PR adds the auto scroll functionality in the sidebar when the user goes to the Doc section. This is achieved by adding a useEffect hook which gets triggered when the activeSlug in the sidebar changes.
Proof of Work
async-sidebar-2023-05-13_23.33.34.mp4
Related issue(s)
Fixes #1582