Adjust row height by font size in PickerIOS

[alin23]

• Explain the motivation for making this change.
• Provide a test plan demonstrating that the code is solid.
• Match the code formatting of the rest of the codebase.
• Target the master branch, NOT a "stable" branch.

Motivation (required)

There is a problem where setting a bigger fontSize in PickerItem style
clips the top and bottom of the text.
This solves that problem by computing the row height using the font
size.

Test Plan (required)

Create a PickerIOS component and set a larger font size (e.g. 50). The row height will grow accordingly.

Example with fontSize=50: Screenshot

[shergin]

How do you choose 8 as a compound padding?

[shergin]

Please fix code style.

[alin23]

@shergin I chose it by trial and error. This value made the view look the most natural.
I tried to find out which is the default margin in UIPickerView but it's not documented and stepping through the disassembled UIKit doesn't show any obvious constant for margin/padding.

[shergin]

Okay, but... Default row height is 40 (Right?), the default font size is 21 (Right?), so 21 + 8 = 21 != 40, so it means we change/break default styling. 🤔
Was it intentional?
Should we use 19 instead of 8?

[alin23]

@shergin you're right, it makes sense. It actually looks way better with a 19 offset

[shergin]

👍

[facebook-github-bot]

@alin23 I tried to find reviewers for this pull request and wanted to ping them to take another look. However, based on the blame information for the files in this pull request I couldn't find any reviewers. This sometimes happens when the files in the pull request are new or don't exist on master anymore. Is this pull request still relevant? If yes could you please rebase? In case you know who has context on this code feel free to mention them in a comment (one person is fine). Thanks for reading and hope you will continue contributing to the project.

[shergin]

@alin23 I am sorry for the delay, should we still merge this? Have tried to test how this change affects default usage?

[alin23]

@shergin Sorry for closing this, I just took for granted what the bot said and thought that the file was removed/refactored somehow.

Yes, I still think this needs to be merged. I got a request from another react-native user to merge this into master so I guess it would be a good idea to do this.

Default usage didn't seem to be affected or changed in any way in my previous tests. As long as I didn't change the default font size, everything looked the same.

[pull-bot]

Attention: @shergin, @javache

Generated by 🚫 dangerJS

[facebook-github-bot]

@shergin has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

[ldhios]

@alin23 why auto row height with font size？ it is correct？ If i use a few PickerIOS ， fontSize is different， row height is catastrophic。 like this image

[alin23]

@ldhios indeed, I didn't take into consideration that the columns could have different font sizes, thus breaking the alignment. It might not be correct, but it is better than clipping the text.

[ldhios]

@alin23 Thanks for you reply in time。 Can you help me fix my issues？

[alin23]

@ldhios I'm sorry but I haven't developed in Obj-C or React Native in a while. A workaround would be to hide those separators by setting the separatorColor to transparent or same as the background color.

[ldhios]

@alin23 Thanks。