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 line height based spacing model #4236

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Enter-tainer
Copy link
Contributor

@Enter-tainer Enter-tainer commented May 23, 2024

fix #1028 fix #4224

The ci is expected to fail since the line height is changed, all of ref images are needed to be updated

TODO:

  • bring back orphan and widows prevention
  • fix TODOs. specifically about the spacing of term/enum/list --> How to calculate row gutter for grids
  • Find a good default for line height in normal text and footnote
  • figure out what it TIGHT_LEADING in math

@peng1999
Copy link
Contributor

  • Find a good default for line height in normal text and footnote

My suggestion is to find a value that keeps the line spacing unchanged in most tests...

@Enter-tainer
Copy link
Contributor Author

Enter-tainer commented May 24, 2024

  • Find a good default for line height in normal text and footnote

My suggestion is to find a value that keeps the line spacing unchanged in most tests...

image

It's impossible. Previously, the line height in typst is...(guess what?) 7.15(leading) + 7.240234375(asc) = 14.390234375pt. And this means that if we want it looks the same, the line-height would be 1.3082031250000001em or so.

But when i set it to 1.3082031250000001, the calculated leading is 7.150000000000002. And if i set it to the next float using np.nextafter, it becomes 7.149999999999999.

Anyway, i think it may not affect test a lot. I will set it to 1.308203125 at this moment

@Enter-tainer Enter-tainer force-pushed the mgt/line-height branch 4 times, most recently from 9052a0f to 9314792 Compare May 25, 2024 07:44
Comment on lines -169 to -182
let leading = if EquationElem::size_in(styles) >= MathSize::Text {
ParElem::leading_in(styles)
} else {
let font_size = scaled_font_size(ctx, styles);
TIGHT_LEADING.at(font_size)
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

anyidea what is tight leading? Is it a workaround or something?

@LuxxxLucy
Copy link
Contributor

LuxxxLucy commented Jun 2, 2024

I just want to comment that #4318 may be a possible predecessor for the line height adjust solution can avoid breaking the test. This PR (#4318) is an attempt to support #2200 (I intend to do this and then refactor draft PR #3953), and I think it should be relevant so I think you might be interested to take a look at it.

@Enter-tainer Enter-tainer force-pushed the mgt/line-height branch 2 times, most recently from 4848e48 to cd35a2e Compare June 4, 2024 15:36
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.

Proposal: change leading option to line-height Large inline equations can overlap with other lines
3 participants