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

render: fixing markdown li measurement #665

Merged
merged 5 commits into from
Jan 16, 2023

Conversation

gavin-ts
Copy link
Contributor

@gavin-ts gavin-ts commented Jan 14, 2023

Summary

Fixes a bug where sometimes the line height wouldn't be used when combining inline elements into a block. Also adds 1px of height for h1 and h2's border bottom.

Details

  • fixes nested lists in markdown render slightly mismeasured #647
  • adds test
  • adds debugging flag to print measured dimensions per element in the tree
  • h1 and h2 s were missing 1px for their border bottom
  • the li element had an inline text entry for itself (20.125) then a nested ol (24), but it should have used the full line height (24) when combining the text with the ol
  • we correctly used the line height when it was only inline elements as children, but we missed the case where it was inline elements followed by a block element
  • cleaned up the code so the logic for combining inline elements is clearer

li before

Screen Shot 2023-01-13 at 8 56 53 PM

li after

Screen Shot 2023-01-13 at 8 56 16 PM

example debugging output

Note: not ideal but still helpful (it goes depth first from body and moves one to the right per depth)
you can inspect the resulting (width, height) and margin top/ bottom values per element (and compare with values in the browser render)

┌--text(51.84375,40.25)
┌-h1(51.84375,50.85) mt:24 mb:16     <- this used to have height 49.85     
┌---text(30.046875,20.125)
┌-----text(30.046875,20.125)
┌----li(62.046875,24) mt:0 mb:0
┌---ol(62.046875,24) mt:0 mb:0
┌--li(94.046875,48) mt:0 mb:0      <- this used to have height 44.125
┌-ul(94.046875,48) mt:0 mb:16     <- so this did too
┌body(94.046875,114.85) mt:24 mb:16

h1 before/after

Screen Shot 2023-01-13 at 9 10 03 PM

@gavin-ts gavin-ts force-pushed the fix-md-li-measurement branch from 5e68523 to b8ce18f Compare January 16, 2023 07:31
@gavin-ts gavin-ts marked this pull request as ready for review January 16, 2023 07:31
@gavin-ts gavin-ts merged commit bd15db4 into terrastruct:master Jan 16, 2023
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.

nested lists in markdown render slightly mismeasured
2 participants