8000 Fix footnote styling by salim-b · Pull Request #69 · thuliteio/doks-core · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Fix footnote styling #69

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

Merged
merged 2 commits into from
Oct 27, 2023
Merged

Fix footnote styling #69

merged 2 commits into from
Oct 27, 2023

Conversation

salim-b
Copy link
Contributor
@salim-b salim-b commented Oct 24, 2023

Summary

Fixes styling of footnotes. The affected CSS rule caused the <sup> element from footnote references to be unclickable and have a line break before it.

Note that I have no clue what purpose the affected CSS rule was originally intended to serve.

Motivation

Fix footnotes in content.

Checks

  • Read Create a Pull Request
  • Supports all screen sizes (if relevant)
  • Supports both light and dark mode (if relevant)
  • Passes npm run test

@h-enk h-enk merged commit 66d7cde into thuliteio:main Oct 27, 2023
@h-enk
Copy link
Member
h-enk commented Oct 27, 2023

Note that I have no clue what purpose the affected CSS rule was originally intended to serve.

It's for positioning the footnote link when clicking the footnote backlink (correction for sticky navbar).

Without it — the footnote link is hidden beneath the (sticky) navbar:

Snag_a019f6

With it — it's visible:

Snag_a04c31

I'll re-add it

h-enk added a commit that referenced this pull request Oct 27, 2023
@h-enk
Copy link
Member
h-enk commented Oct 27, 2023

3169e97

@salim-b
Copy link
Contributor Author
salim-b commented Oct 27, 2023

I'll re-add it

But you added the exact same rule as this PR removed? 🤨

Did you test the rule with actual footnote content?

The problem I referred to is content like the following paragraph:

Some paragraph with a footnote reference[^1] in the middle.

[^1]: A footnote.

This paragraph is split (line wrap) after "referen 8000 ce" in the rendered HTML.

@salim-b
Copy link
Contributor Author
salim-b commented Oct 27, 2023

Plus: the problem your CSS rule is intended to solve applies to all anchor links (fragment identifiers), not just footnotes, right? So I guess the sup CSS selector is not what we want to leverage here, but a instead. No?

@h-enk
Copy link
Member
h-enk commented Oct 27, 2023

This paragraph is split (line wrap) after "reference" in the rendered HTML.

Ah, I didn't get that — I see now

Snag_4811f3

@h-enk
Copy link
Member
h-enk commented Oct 27, 2023

Plus: the problem your CSS rule is intended to solve applies to all anchor links (fragment identifiers), not just footnotes, right? So I guess the sup CSS selector is not what we want to leverage here, but a instead. No?

Only <sup> elements with an id attribute are affected — I think that's safe to do. So something like e.g. Y = X<sup>2</sup> is not hit:

Snag_4ce5da

@h-enk
Copy link
Member
h-enk commented Oct 27, 2023

Will look into a fix

@h-enk
Copy link
Member
h-enk commented Oct 27, 2023

This is way more elegant (just a oneliner) and robust :

sup[id] {
  scroll-margin-top: 5rem; 
}

Resources: One line CSS solution to prevent anchor links from scrolling behind a sticky or fixed header

h-enk added a commit that referenced this pull request Oct 27, 2023
@salim-b
Copy link
Contributor Author
salim-b commented Oct 27, 2023

This is way more elegant (just a oneliner) and robust

Awesome, thank you! I didn't know about scroll-margin until now.

@salim-b salim-b deleted the fix-footnotes branch October 27, 2023 14:47
@h-enk
Copy link
Member
h-enk commented Oct 27, 2023

Awesome, thank you! I didn't know about scroll-margin until now.

Me neither 😄

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

Successfully merging this pull request may close these issues.

2 participants
0