Fix sticky sidebar issues #788
Conversation
|
I forgot to mention, please scroll the docs page with some of the articles expanded, in a long page with browser height set to 400px for example. The sidebar should stay in place while you scroll until it meets the header. The code is implemented in a way that the two fragments stay independent and ignorant of each other. The content fragment only applies the scroll code if there is an active sidebar and should not break if there is a navbar or not. |
|
Fixed the bug that was causing the failing test and then fixed a faulty test that failed occasionally. It was failing for this PR so pushed the commit here, let me know if you want me to separate it. |
| {{- if .Params.sticky }} | ||
| <style> | ||
| :root { | ||
| --header-height: 56px; | ||
| } | ||
| </style> | ||
| {{- end -}} |
There was a problem hiding this comment.
Not sure I like the hardcoding of that. Can't think of a better solution so.
There was a problem hiding this comment.
I can't think of a solution that doesn't involve JS and is not hard coding the value. I'll try to figure something out and maybe we can create an issue to track it.
There was a problem hiding this comment.
Fine to keep it as is and open a "revisit" to track this.
|
Like the scrolling, but the position doesn't seem perfect. This is more a side effect of #513 I guess. As we are overwriting the height. Would that be something we would like to fix here as well or am I missing something?
|
|
Edgecase, that should probably get a dev/ page and maybe a test. What happens with 2 sidebars? Do we need an additional note for the "sticky" config for the content fragment? |
|
What do you think about a dev/ page for manual testing and probably also automated testing? |
|
@stp-ip Added tests and dev pages accordingly. |
What this PR does / why we need it:
Which issue this PR fixes:
fixes #700
fixes #601
Release note: