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

Customize line height for headings #535

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

tomekzaw
Copy link
Collaborator

@tomekzaw tomekzaw commented Nov 5, 2024

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

Manual Tests

Linked PRs

@tomekzaw
Copy link
Collaborator Author

tomekzaw commented Nov 5, 2024

TODO: check markdownStyle={{ h1: { lineHeight: undefined }}}

@SzymczakJ
Copy link
Contributor

I found some inconsistencies between iOS and Android platforms:
line height ios
android line height

h1 text on Android is centered, on the other hand on iOS it is justified to the bottom of the line

@SzymczakJ
Copy link
Contributor

SzymczakJ commented Nov 19, 2024

These are test screenshots to make sure this PR doesn't break anything on main(left - state of main, right - state of PR):

  • iOS
Screenshot 2024-11-19 at 16 06 07 ss - text headin text Screenshot 2024-11-19 at 16 06 31 ss - text headin3 text Screenshot 2024-11-19 at 16 06 54 ss - text headinB text Screenshot 2024-11-19 at 16 07 11 ss - text headin3B text
  • Android
Screenshot 2024-11-19 at 15 52 05 and - text headin text Screenshot 2024-11-19 at 15 52 37 and - text headin3 text Screenshot 2024-11-19 at 15 53 20 and - text headinB text Screenshot 2024-11-19 at 15 53 41 and - text headin3B text

@tomekzaw
Copy link
Collaborator Author

Related change in react-native: facebook/react-native#47271

@tomekzaw
Copy link
Collaborator Author

tomekzaw commented Dec 7, 2024

TODO: heading line height when nested in blockquote on iOS

Screenshot 2024-12-07 at 12 37 37

@tomekzaw
Copy link
Collaborator Author

tomekzaw commented Dec 7, 2024

TODO: center text vertically on iOS

Screenshot 2024-12-07 at 12 39 20

@tomekzaw
Copy link
Collaborator Author

tomekzaw commented Dec 7, 2024

Android is okay

Screenshot 2024-12-07 at 12 40 58

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.

Simplify MarkdownLineHeightSpan logic
2 participants