Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable announcement banner to be sticky #546

Open
1 task done
alexisthual opened this issue Oct 9, 2022 · 2 comments
Open
1 task done

Enable announcement banner to be sticky #546

alexisthual opened this issue Oct 9, 2022 · 2 comments
Labels
enhancement New feature or request

Comments

@alexisthual
Copy link

What's happening?

Currently, I use the announcement banner to warn users that they are looking at an outdated version of the generated docs.
However, the banner is not sticky / fixed, so it disappears when users scroll down the page. This can be problematic when users access the docs through a hyperlink referencing a paragraph, as they probably won't see the banner.

I tried using custom CSS to simply make the banner sticky, but it seems it seems more complex (see discussion #545).
In particular, the rest of the page will scroll under the banner, eventually hiding the logo, which I think is not the expected behaviour.
Screenshot from 2022-10-08 18-24-04

Reproducer

Simply use announcement option and scroll down.

Expectation

In my opinion, banners should be sticky by default in order to prevent this, but I guess having an option to make them sticky or not could be nice! 🙂

Code of Conduct

@pradyunsg pradyunsg added the enhancement New feature or request label Oct 10, 2022
@davidism
Copy link
Contributor

I would like to have this as well. I'm hoping to adopt Furo as the base theme for Pallets projects and Flask.

We have a sticky banner when viewing old or dev docs, you can see an example here: https://flask.palletsprojects.com/en/2.2.x/views/#view-decorators. It's sticky so that no matter where a user is linked to in the docs, they see the banner with a link to the current docs. I wrote some JavaScript to prevent it from overlapping headers when linking to an anchor as well as when clicking anchors on the page: https://github.com/pallets/pallets-sphinx-themes/blob/529e8fc362d99f600057fe78ffbe24a11f442276/src/pallets_sphinx_themes/themes/pocoo/static/version_warning_offset.js. It works on large and small screen sizes.

I tried using Furo's announcement for this, but when I tried to make it sticky I ran into a few different problems:

  • Unless I move the element inside the page div (instead of a sibling), it eventually scrolls away (I think when the sidebar conents height is reached, but not sure).
  • At small screen sizes, Furo makes its header sticky. This needs to be pushed down with top: var(--header-height), otherwise it overlaps the announcement.
  • It overlaps headers when navigating to or clicking anchors. Furo's sticky header doesn't overlap, I think gumshoe.js is handling this. Gumshoe is initialized with a function that calculates the height, so it would need to be adjusted similarly to the previous point to account for both the sticky header and announcement.

@Eric-Arellano
Copy link
Contributor

Eric-Arellano commented Jun 20, 2023

FYI this is the CSS we needed to add a sticky top nav bar to qiskit_sphinx_theme, which builds off of Furo:

https://github.com/Qiskit/qiskit_sphinx_theme/blob/765c936a6b285c537e8cb9247d4b84862b8417a8/qiskit_sphinx_theme/furo/base/static/styles/qiskit_changes.css#L121-L175

HTML change:

https://github.com/Qiskit/qiskit_sphinx_theme/blob/765c936a6b285c537e8cb9247d4b84862b8417a8/qiskit_sphinx_theme/furo/base/page.html#L9-L11

We use these visual regression tests from Playwright to make sure we aren't breaking things. That is, these are the edge cases we found that a sticky top element breaks:

https://github.com/Qiskit/qiskit_sphinx_theme/blob/765c936a6b285c537e8cb9247d4b84862b8417a8/tests/js/snapshots.test.js#L60-L139

And indeed, gumshoe.js is broken for detecting the current section in the right side bar. But we're hoping that @pradyunsg will be okay with merging #664, which would fix the problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants