Implement support for sticky banners and utilize them

[pradyunsg]

My motivation with this is to make the Packaging PEPs more clearly point readers to the canonical location (i.e. packaging.python.org) rather than the PEPs that introduced them. :)

This also makes some progress toward #2719, by implementing one of the approaches discussed there.

PEP 621 is a good PEP to try to see how this behaves: https://pep-previews--2992.org.readthedocs.build/pep-0621/

[CAM-Gerlach]

Thanks @pradyunsg for helping out on this! I'd meant to get to this previously, but it just slipped through the cracks.

In testing this, sometimes when loading a page with a fragment identifier (the original scenario motivating this change), at least for me in FF on Windows, the scroll offset isn't applied and the banner covers the heading, which I'm guessing is because the scroll event races this snippit on DOMContentLoaded and sometimes the scroll is executed before the scroll offset has been set.

To avoid this, you could add something like the following at the end of the JS snippit:

if (location.hash.length > 1) {
    document.getElementById(location.hash.substring(1)).scrollIntoView();
}

I'm also experiencing the offset height sometimes being too much on Firefox mobile on a Pixel 3a latest Android, at least on first uncached page load, whereas subsequent cached loads fixes it. I'm thinking this might be due to the stylesheet/other assets not being already fully loaded and rendered that results in a different calculated height for the banner at the time of DOMContentLoaded than eventually gets applied, which changing it to load might fix if so.

[hugovk]

Thanks for this!

It's working on mobile too, but takes up quite a bit more space. On my Samsung S10:

Maybe once it becomes sticky, it could shrink and show a shorter message?

---

This doesn't happen on my phone, but does when testing other layouts using desktop Chrome (e.g. iPhone SE and Pixel 5):

Click on a header to go to https://pep-previews--2992.org.readthedocs.build/pep-0621/#rationale and the Rationale header text is obscured by the sticky header:

---

Finally, this might just be a problem with desktop Chrome's mobile renderer, but there's a single line of pixels shown above the sticky header, showing a tiny slice of the body text:

As you scroll, that pixel line flickers as the text goes past. The fix would be to change top: 0 to top: -1px, but I don't know if this occurs in real devices?

[CAM-Gerlach]

Thanks for the detailed further testing and screenshots!

Maybe once it becomes sticky, it could shrink and show a shorter message?

Hmm, well that would require more extensive changes, and wouldn't serve the primary use case of making it sticky (so it shows up when deep-linking inside PEPs) as users would never see the full thing. What about adding a close button (X) that once clicked would disable the sticky behavior? This would also resolve a lot of my concerns about it being too distracting for people reading the PEPs that really mean to.

This doesn't happen on my phone, but does when testing other layouts using desktop Chrome (e.g. iPhone SE and Pixel 5):

This is because the sticky height is only calculated once on page load, so unless you do a full reload after switching device profiles, the height won't update. I naturally see this also on FF desktop's mobile renderer, but it goes away when reloading, and won't happen on a real device, of course (and I can also confirm it worked, with the one sporadic issue above, in both FF and Chrome on my Pixel 3a).

Finally, this might just be a problem with desktop Chrome's mobile renderer, but there's a single line of pixels shown above the sticky header, showing a tiny slice of the body text:

I can't repro this in either Chrome or FF on my Pixel 3a, nor on FF desktop's mobile renderer with the profiles of the devices you mentioned (iPhone SE and Pixel 5). I strongly suspect this just has to do with the precise framing of the simulted mobile viewport not exactly matching what would be seen on a real phone's screen.

[pradyunsg]

The fix would be to change top: 0 to top: -1px, but I don't know if this occurs in real devices?

Can't reproduce this on BrowserStack's "real devices" rendering -- so... I'll say this is a bug in Chrome. :)

[CAM-Gerlach]

@pradyunsg what do you think about

What about adding a close button (X) that once clicked would "close" the sticky (i.e. JS event handler attached to the element onClick event that removes the sticky CSS class from parent)? This would also resolve a lot of my concerns about it being too distracting for people reading the PEPs that really mean to.

[pradyunsg]

What about adding a close button (X) that once clicked would "close" the sticky (i.e. JS event handler attached to the element onClick event that removes the sticky CSS class from parent)? This would also resolve a lot of my concerns about it being too distracting for people reading the PEPs that really mean to.

I like the sound of this -- I think it's OK for us to do this in a follow-up, because that's a significant amount of complexity on its own.

[pradyunsg]

Maybe once it becomes sticky, it could shrink and show a shorter message?

I think the close button is good-enough as a solution for this TBH, because it's a more respectful way to allow users to hide banners as-and-when they wish to do so.

[pradyunsg]

sometimes when loading a page with a fragment identifier (the original scenario motivating this change), at least for me in FF on Windows

I've tried multiple attempts at reproducing this and failed to do so. I've also added a load event handler, to cover for your concern around when CSS rendering happens. I don't have notes on what the ordering of scroll vs render vs events is on different browsers and having an extra no-op function call seems harmless-enough -- so let's just handle both of these events and it should hopefulyl cover your concerns. If not, let's investigate that separately since this seems to be working in the 80%/90% case. :)

One thing that came up in manual testing for this was the need for handling resizing, and I've logic to cover that as well.

TBH, I'm leaning toward good-enough + done is better than perfect + not-done -- I suggest that we improve this further in follow ups if there are not show-stopping concerns here; rather than having this go through multiple cycles to do it perfectly in the first cut. :)

[hugovk]

Thank you!

[CAM-Gerlach]

Thanks for all your help with implementing this and getting this over the line, @pradyunsg , as well as putting up with all our nitpicks and review feedback. I look forward to using this feature in the days ahead.

[hugovk]

Looks like this is causing a problem for PEP 12 only, in that it creates an offset for a banner that doesn't exist.

To reproduce online:

1. Open https://peps.python.org/pep-0012/
2. Click any anchor link in the Contents, for example click "General" (https://peps.python.org/pep-0012/#general) and you an offset for a sticky banner that is not in use, meaning "General" is not flush with the top of the page

To reproduce locally:

make clean dirhtml
python -m http.server -d build

2. Open http://[::]:8000/pep-0012/
3. Click any anchor link in the Contents (http://[::]:8000/pep-0012/#general)

Can reproduce with main, 06e5a8a from this PR, but not the preceding commit 40f5796.