search

google / docsy

A set of Hugo doc templates for launching open source content.
https://docsy.dev
Apache License 2.0
2.51k stars 877 forks source link

Problem with scroll-anchoring #1116

Open richvdh opened 1 year ago

richvdh commented 1 year ago

Using Firefox 102.0.1, and Google Chrome 103.0.5060.134:

The height: 100vh property on .td-outer (link) seems to break scroll-anchoring, so that resizing the window causes the content to scroll to a completely different location. Removing that property restores scroll-anchoring (and doesn't seem to have any other detrimental effects, at least on our site).

It appears that the change in height counts as a suppression trigger: setting layout.css.scroll-anchoring.suppressions.enabled to false makes scroll anchoring work as expected.

LisaFC commented 1 year ago

Huh, well spotted. Do you want to create a PR to change that setting for Docsy and we can see how/if it does anything unwanted on our docs site?

richvdh commented 1 year ago

I'm very happy to (#1128), though I have to say, I'm far from a CSS expert. I suspect that setting must be there for a reason, and its removal is likely to cause breakage...

LisaFC commented 1 year ago

@chalin do you want to take a look? I'm playing around with the window size in preview and it doesn't seem to have done anything weird, but I'm not a CSS expert either.

chalin commented 1 year ago

@thisisobate - could you take a look? Thanks. /cc @nate-double-u

thisisobate commented 1 year ago

@chalin I tried toggling off the height attribute via the chrome developer console but resizing the window still causes the content to scroll to a different location. Case in point, docsy.dev.

I think scroll-anchoring behaviors are meant to occur when a user scrolls to read content; I don't really know if it's affected by a window resize too.

Regardless, removing the height doesn't break or add any noticeable change to the current UI so it's safe to merge to production.

chalin commented 1 year ago

The height is there to force the footer to be positioned at the bottom of the page (in wide views), so we can't remove it. Visit the Community page and toggle the height, you'll see what I mean.

I agree that there might be something to be addressed here, but just removing height: 100vh isn't it. I'll need to investigate further. Comment and ideas are welcome.

chalin commented 1 year ago

Interesting hits from this query for me (FYI): https://www.google.com/search?q=why+use+a+css+height+of+100vh

richvdh commented 1 year ago

The height is there to force the footer to be positioned at the bottom of the page (in wide views), so we can't remove it. Visit the Community page and toggle the height, you'll see what I mean.

I agree that there might be something to be addressed here, but just removing height: 100vh isn't it. I'll need to investigate further. Comment and ideas are welcome.

Yes, that makes sense. Unfortunately this is now out of the range of my limited CSS knowledge, so I'm going to close my PR and let people who know what they are doing find a better solution here :)

chalin commented 1 year ago

@richvdh - I tried out https://github.com/matrix-org/matrix-spec/pull/1183, and it doesn't seem to fix the problem. We can keep this issue open, but I won't have time to investigate for a while. @thisisobate if you have more ideas, let us know.

richvdh commented 1 year ago

@richvdh - I tried out matrix-org/matrix-spec#1183, and it doesn't seem to fix the problem.

I can imagine that on other sites, there might be other problems, but empirically, https://github.com/matrix-org/matrix-spec/pull/1183 seems to work fine for me on our particular site! Did you try it on our site (https://pr1183--matrix-spec-previews.netlify.app/), or another?

ShlokJswl commented 1 year ago

Hello can you assign this issue to me?

emckean commented 1 year ago

Hi @ShlokJswl we don't assign issues in this project. If you would like to contribute please check out our Contributing Guidelines.