Docs layout issues when announcementBar visible

[slorber]

Have you read the Contributing Guidelines on issues?

• I have read the Contributing Guidelines on issues.

Prerequisites

• I'm using the latest version of Docusaurus.
• I have tried the npm run clear or yarn clear command.
• I have tried rm -rf node_modules yarn.lock package-lock.json and re-installing packages.
• I have tried creating a repro with https://new.docusaurus.io.
• I have read the console error message carefully (if applicable).

Description

When the announcementBar is visible, the docs sidebar has a weird layout issue

Page: https://docusaurus.io/tests/docs

Good URLs to reproduce (long sidebar collapsed - long content):

• https://docusaurus.io/docs/sidebar/items
• https://docusaurus.io/tests/docs/toc/toc-_-5/

Note: it's not because of the hideable sidebar feature, the issue is also there when there's no sidebar collapse button

Also related to #7815: the announcement bar height should probably be dynamic/flexible and layout should rather adapt gracefully to any size.

Reproducible demo

No response

Steps to reproduce

https://docusaurus.io/tests/docs

Expected behavior

No weird layout with elements overlapping

The visible scrollbar takes the appropriate height.

Actual behavior

Elements overlap and the scrollbar does not go up to the bottom (the scrollbar should touch the collapse button)

Your environment

• Public source code:
• Public site URL:
• Docusaurus version used:
• Environment name and version (e.g. Chrome 89, Node.js 16.4):
• Operating system and version (e.g. Ubuntu 20.04.2 LTS):

Self-service

• I'd be willing to fix this bug myself.

[r-lawrence]

Hi everyone!
I have investigated this issue and believe I have created a fix in this PR. It appears that a CSS property of sticky is causing the overlapping issue seen in the above screenshot. I have removed the CSS property and was hoping to get some reviews. Thanks a bunch!

[msaini28r]

@slorber I would like to work on this issue if it is available.

[Djunnni]

i will try this issue with #8374
#8377 (comment)

If I'm too late, someone else can do it!

[arbitrarymahi]

Hello @slorber, I have found the solution to this problem, How do I make sure the fix doesn't create other issues, can you please help, it's my first time contributing.

[slorber]

@arbitrarymahi you can create a pull-request and we'll review it.

Note: all attempts to fix this have failed so far, it's not an easy issue to take for a first contribution so make sure you tested all the edge cases.

[arbitrarymahi]

@slorber Please check my PR for this issue: #8829

[slorber]

Hi community

Thanks for all contributors that gave it a try to solve this annoying layout problem.

We received multiple PRs but no implementation provided was satisfying enough.

All the proposed PRs had bugs in the review URLs I suggested:

• docusaurus.io/docs/sidebar/items
• docusaurus.io/tests/docs/toc/toc-_-5

This problem is likely more complex to solve than it seems, and I'll probably have to handle it myself instead of asking for community help.

For now, I suggest that the community step back on this issue and stop submitting PRs. I'm going to do the cleanup and close the existing PR attempts.

If you still think you can solve it properly and submit a PR, please review carefully the 2 URLs above so that there is no issue at all (we are not looking for a partial fix here) on different viewport sizes with/without the announcement bar. I'll review these pages very carefully on your PR so doing this work upfront ensures that your PR is valid.

[r-lawrence]

Hi Everyone!

I took another stab at this issue. Initially I was only looking at CSS functionality which was the incorrect approach. When the announcement bar is shown, the root of the problem is we need to adjust the height dynamically for the entire sidebar. The details of everything are included in this PR.

Additionally I have one question regarding argos continuous integration check. It appears based on my changes, elements positions changed. Looking at the screenshots from the CI, the changes appear to be minor. I've never used argos before and I would like someone else to look over them to ensure I didn't miss anything, before I mark them as ok.

Thanks a bunch for taking the time to look this over and let me know if any other changes should be made!

[slorber]

Initially I was only looking at CSS functionality which was the incorrect approach.

@r-lawrence will review your PR on time but please make sure your solution works BEFORE react hydration. If it's using hooks it's likely working only after hydration.

For me, the solution is rather CSS math + inlined JS, and I'll review carefully that it works as I expect it to. Please test your PR with slow 3G mode activated and ensure there are no unexpected layout shifts during the React hydration, and that it works fine if we attempt to scroll before the hydration.

---

Argos CI is new, I'll review the UI diff reported, but wonder why your PR should involve minor changes at all.

before I mark them as ok.

Are you able to do that? This is unexpected.

[r-lawrence]

will review your PR on time but please make sure your solution works BEFORE react hydration. If it's using hooks it's likely working only after hydration.

I didn't have to much time to take a look at this right now, but I think you are correct. When running the functionality on 3G slow throttled mode in the chrome dev tools, there is a small layout shift. You can see it in this short video. The bottom sidebar ad and button are initially located incorrectly because the hydration has not occurred and the application has not detected the announcement bar. Then it adjusts its positioning, after the hooks events fire.

Screen.Recording.2023-05-10.at.10.31.52.AM.mp4

• I think I may be a bit confused... How does the announcement bar show up before hydration? It appears it managed by the useContext hook listed above. Is there a way we can somehow mimic this functionality for the sidebar too? IE - allowing for the application to detect if the announcement bar is present before hydration and displaying the proper height for the sidebar. Then using the hook to correctly adjust height based on scroll movement?
  • do you have any suggestions on how this can be done with strict CSS math + inline JS? I'de be happy to give them a shot.

Also, originally the functionality that used the useAnnouncementBar function from packages/docusaurus-theme-common/src/contexts/announcementBar.tsx was housed in packages/docusaurus-theme-classic/src/theme/DocSidebar/Desktop/Content/index.tsx. To me, it appeared this functionality was added in an effort to fix the sidebar issue when the announcement bar was present. This functionality is what causes the margin to be off in the sidebar when the announcement bar is present. In turn, this also affects the scrollbar not extending to the bottom of the sidebar. (see screenshots below)

• Even if this solution isn't viable, should we still remove the functionality added to packages/docusaurus-theme-classic/src/theme/DocSidebar/Desktop/Content/index.tsx to add margin when the announcement bar is detected? This is causing the bug thats related to the scrollbar not extending to the bottom of the sidebar ad.

Argos CI is new, I'll review the UI diff reported, but wonder why your PR should involve minor changes at all.

Looking at this quickly, some css was changed to allow for the sidebar ad and button to always be positioned at the bottom of the page. The resulting UI changes could be from some of this CSS. There was two places I changed css for height - packages/docusaurus-theme-classic/src/theme/DocRoot/Layout/Sidebar/styles.module.css and packages/docusaurus-theme-classic/src/theme/DocSidebar/Desktop/styles.module.css. One of these changes could be causing the layout shift in Argos?

Thanks for your response and when I have additional time later, I'll be happy to continue looking at this for a bit!

[slorber]

@r-lawrence I'm not super familiar with this code and can't help you much. It is there for a long time, was not coded by me.

I am not 100% sure of the solution to use and how many changes are required. If I help you figure things out, I would be faster to actually solve the problem myself.

What I'm sure of is the outcome we want: 0 layout shifts.
And for that, I believe reading localStorage in inlined JS + using CSS math could work. not sure but it's what I would try first.

If you are able to solve it on your own that's nice, otherwise, I can't really help and it's preferable that you close your PR.

[r-lawrence]

@slorber Hey no problem. I'll work on this a bit later when I have some more time. If I can't come up with a solution in a reasonable time, I'll go ahead and close the PR. Thanks!

[ArchitGajjar]

Hi @slorber - is this issue still open ? I would like to give a try!