Customize line height for headings

[tomekzaw]

Details

This PR adds h1.lineHeight to markdownStyle. If not set, MarkdownTextInput will use OS-default line height (appropriate to h1.fontSize). If set, this will override OS-default line height.

Looks like Codegen still doesn't have proper support for Float | undefined (and neither for Float | null) so I had to go with -1 as the default value.

Closes #522.

Related Issues

#522
Expensify/App#52827 (comment)

Manual Tests

Linked PRs

[tomekzaw]

TODO: fix font size or line height when adding newline after heading (reproducible on 7ceeadc)

[tomekzaw]

Test cases with screenshots from 7ceeadc:

• without lineHeight and without h1.lineHeight ✅
  
• with lineHeight: 30 and without h1.lineHeight ✅
  
• without lineHeight and with h1.lineHeight: 60 ⚠️ requires patch to react-native
  
• with lineHeight: 30 and with h1.lineHeight: 60 ✅
  
• h1 nested in blockquote ✅
  
• h1 nested in double blockquote ✅
  
• line height in new line after h1 ❌
  

[tomekzaw]

The absence of react-native patch makes a difference only in a case when lineHeight is not set but h1.lineHeight is set. The behavior of MarkdownTextInput is identical to RN's built-in TextInput (notice the incorrect height resulting in visible padding below the text).

without patch	with patch

<TextInput
  multiline
  style={{borderWidth: 1, width: 300, fontSize: 20}}
  ref={ref}
  placeholder="Type here...">
  First line{'\n'}
  <Text style={{fontSize: 30, lineHeight: 60}}>Second line</Text>
  {'\n'}Third line{'\n'}Fourth line
  {'\n'}
  Fifth line
</TextInput>

---

Setting lineHeight in TextInput fixes the problem:

[tomekzaw]

TODO: fix placeholder vertical alignment on iOS

[tomekzaw]

TODO: fix cursor position when text is empty and lineHeight: 30 is set

edit: it works without MarkdownTextInputDelegate 😢

[tomekzaw]

Waiting for facebook/react-native#51344.

[rohit9625]

Hi @tomekzaw
I saw that your PR is merged to the React Native. Are we still on hold?

[tomekzaw]

Hi @rohit9625, yes, the PR facebook/react-native#51344 indeed was merged. However, this PR still needs some more work on it to address some minor issues like those reported in the comments above.

[rohit9625]

So, are you working on them? Let me know if you need any help with that :)

[tomekzaw]

Someone from the Live Markdown team at Software Mansion will take care of it.