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

nicer style rules for margin around footnote defs #2524

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

WaffleLapkin
Copy link
Member

@WaffleLapkin WaffleLapkin commented Jan 15, 2025

previous implementation used :not(.footnote-definition) + .footnote-definition and .footnote-definition + :not(.footnote-definition).
the latter selector caused many problems:

  • it doesn't select footnote definitions which are last children (this can be easily triggered in a blockquote)
  • it changes the margin of the next sibling, rather than the footnote definition itself, which can also shrink margin for elements with big margins (this happens to headings)
  • because it applies to the next sibling it is also quite hard to override in user styles, since it may apply to any element

this PR replaces the latter selector with :not(:has(+ .fd)), which fixes all of the mentioned problems.

see rust-lang/reference@bc02bf9 for concrete motivation.

See https://caniuse.com/mdn-css_selectors_not and https://caniuse.com/css-has for comparison of browser support of :not and :has (:has is slightly worse).

cc @ehuss

@rustbot rustbot added the S-waiting-on-review Status: waiting on a review label Jan 15, 2025
@ehuss
Copy link
Contributor

ehuss commented Jan 17, 2025

I don't think this works correctly. The problem is that if you have multiple sections with footnotes, only the footnotes in the first section will get the styling applied. You should be able to see the differences in something like:

# Chapter 1

This is a footnote in the first section[^1].

[^1]: Testing

## Next section

This is a footnote in the second section[^2].

[^1]: Testing

## Last section

Test

previous implementation used `:not(.fd) + .fd` and `.fd + :not(.fd)`.
the latter selector caused many problems:
- it doesn't select footnote defs which are last children
  (this can be easily triggered in a blockquote)
- it changes the margin of the next sibling, rather than the footnote def
  itself, which can also *shrink* margin for elements with big margins
  (this happens to headings)
- because it applies to the next sibling it is also quite hard to
  override in user styles, since it may apply to any element
  
this commit replaces the latter selector with `:not(:has(+ .fd))`,
which fixes all of the mentioned problems.
@WaffleLapkin WaffleLapkin force-pushed the first-last-of-type-footnote branch from 1bffe0f to 64cca13 Compare January 21, 2025 00:22
@WaffleLapkin
Copy link
Member Author

WaffleLapkin commented Jan 21, 2025

Ugh, I had a wrong understanding of what :last-of-type means, I though "sibling" meant "+", not "~"...

I changed the rule to :not(:has(.footnote-def))), which does do what I wanted. It actually slightly changes the style (new on the right):
image

The two differences are:

  • More margin before headings after footnote defs (before footnote rule would override headings' margins to be smaller)
  • More margin if the last footnote def has no following siblings (i.e. it's the last child)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-review Status: waiting on a review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants