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

fixes anchor appearing bellow search bar #1051

Merged
merged 2 commits into from
Dec 7, 2023

Conversation

EmileTrotignon
Copy link
Collaborator

It used to be that the when following a link to an anchor, the anchor would appear a the top of the view, which was under the new search bar. This adds a margin of a 100px to such jumps.

@EmileTrotignon EmileTrotignon added this to the 2.4.0 milestone Dec 5, 2023
@EmileTrotignon EmileTrotignon added the no changelog This pull request does not need a changelog entry label Dec 5, 2023
Copy link
Collaborator

@Julow Julow left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Needs a rebase.

@@ -95,7 +95,10 @@

:root,
.light:root {
--main-background: #FFFFFF;

scroll-padding-top: 100px;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this be in em, like the font size and the margin of items ?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

More than a specific unit, shouldn't this be computed from --search-bar-height and --search-padding-top to be sure it is enough? Something like:

Suggested change
scroll-padding-top: 100px;
scroll-padding-top: calc(var(--search-bar-height) + var(--search-padding-top) + 20px);

Where 20px can be replaced by a size in em if needed.

@panglesd panglesd merged commit d32cd0c into ocaml:master Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog This pull request does not need a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants