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

Use -webkit-text-size-adjust instead of unsetting inline-block #202

Merged
merged 3 commits into from
Nov 19, 2024

Conversation

jez
Copy link
Contributor

@jez jez commented Nov 18, 2024

Having display: inline-block; is useful. I frequently customize the
pandoc code block styles to allow for line highlights (see 1).
The CSS styles for those line hightlights rely on the enclosing span
for a line being able to span the full width of the code block, which is
not possible without making the span be display: inline-block;.

The original issue reported in jgm/pandoc#7248
was that specifically iOS Safari renders font sizes incorrectly.

This is a known iOS bug. I first discovered the workaround for it while
working with the tufte-css project (see 2), and similar workarounds
are implemented in various CSS normalize/reset projects.

I have verified that including this -webkit-text-size-adjust property
fixes the original bug reported against pandoc. Given that the solution
in this PR allows keeping display: inline-block; which has its own
uses, fixes the original bug, and is an iOS-specific solution to an
iOS-specific problem, I believe that this is a more compelling long-term
solution (at least until iOS decides to change this behavior).

I, of course, defer to the maintainers of the skylighting project as to
whether you want this change. I've already worked around this problem in
my own projects that consume the output of pandoc- and
skylighting-highlighted code blocks (by basically vendoring these
changes into those projects), so I am not personally blocked. I'm
contributing this change upstream in the hopes that it might help
unblock others.


Related prior work:

jez added 3 commits November 18, 2024 13:58
Having `display: inline-block;` is useful. I frequently customize the
pandoc code block styles to allow for line highlights (see [1]).

The CSS styles for those line hightlights rely on the enclosing `span`
for a line being able to span the full width of the code block, which is
not possible without making the `span` be `display: inline-block;`.

The original issue reported in jgm/pandoc#7248
was that specifically iOS Safari renders font sizes incorrectly.

This is a known iOS bug. I first discovered the workaround for it while
working with the tufte-css project (see [2]), and similar workarounds
are implemented in various CSS normalize/reset projects.

I have verified that including this `-webkit-text-size-adjust` property
fixes the original bug reported against pandoc. Given that the solution
in this PR allows keeping `display: inline-block;` which has its own
uses, fixes the original bug, and is an iOS-specific solution to an
iOS-specific problem, I believe that this is a more compelling long-term
solution (at least until iOS decides to change this behavior).

I, of course, defer to the maintainers of the skylighting project as to
whether you want this change. I've already worked around this problem in
my own projects that consume the output of pandoc- and
skylighting-highlighted code blocks (by basically vendoring these
changes into those projects), so I am not personally blocked. I'm
contributing this change upstream in the hopes that it might help
unblock others.

[1]: https://jez.io/pandoc-markdown-css-theme/features/#numbered-and-highlighted-lines
[2]: edwardtufte/tufte-css#81 (comment)
jez added a commit to sorbet/sorbet.run that referenced this pull request Nov 18, 2024
See jgm/skylighting#202 for an explanation why
we ended up in this situation, and
5dc8050 for what the difference in
generated CSS looks like for codeblocks in new vs old pandoc versions.
@jgm jgm merged commit 260a53d into jgm:master Nov 19, 2024
8 checks passed
@jgm
Copy link
Owner

jgm commented Nov 19, 2024

Thanks, I'm glad to have a better fix!

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