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

Update RCTUITextView to use the systemFontSize for macOS #1836

Merged
merged 2 commits into from
May 31, 2023

Conversation

lyzhan7
Copy link

@lyzhan7 lyzhan7 commented May 31, 2023

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary

We have a component using RCTUITextView, and the placeholder text looks too big on macOS. The placeholder text font size was hardcoded to 17px (which according to this is the iOS default font size for body text), but for macOS it should be the system font size, i.e. 13px.

Changelog

[macOS] [FIXED] - Update RCTUITextView to use the systemFontSize

Test Plan

- macOS iOS (no change)
Before macOS before iOS before
After macOS after iOS after

Also confirmed from debugging that the font size on macOS is indeed 13px

@lyzhan7 lyzhan7 requested a review from a team as a code owner May 31, 2023 20:52
@lyzhan7 lyzhan7 merged commit f469e72 into microsoft:main May 31, 2023
@lyzhan7 lyzhan7 deleted the textview-font branch May 31, 2023 22:05
lyzhan7 added a commit to lyzhan7/react-native-macos that referenced this pull request Jun 1, 2023
)

* Update RCTUITextView to use the systemFontSize for macOS

* ifdef the function declaration as well
lyzhan7 added a commit that referenced this pull request Jun 1, 2023
* Update RCTUITextView to use the systemFontSize for macOS

* ifdef the function declaration as well
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.

3 participants