Summary:
- [x] Explain the **motivation** for making this change.
- [x] Provide a **test plan** demonstrating that the code is solid.
- [x] Match the **code formatting** of the rest of the codebase.
- [x] Target the `master` branch, NOT a "stable" branch.
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.
Create a PickerIOS component and set a larger font size (e.g. 50). The row height will grow accordingly.
Example with `fontSize=50`: [Screenshot](http://i.imgur.com/YwK5fOc.png)
Closes https://github.com/facebook/react-native/pull/13513
Differential Revision: D5692124
Pulled By: shergin
fbshipit-source-id: 4629403e37ad68cdbc0b17b48ba924a77e133078
Summary:
This PR fixes a declaration mismatch for `YGNodeCanUseCachedMeasurement` where the last argument is declared non `const` in `.h` and `const` in `.c`.
Additionally it uses explicit `float` for fraction calculation do avoid usage of `double` assignment.
Closes https://github.com/facebook/yoga/pull/607
Differential Revision: D5677931
Pulled By: emilsjolander
fbshipit-source-id: 502da957089e4439ed956987ff8dec10bd033ba3
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.
You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html
Happy contributing!
-->
I was trying to typeset mathematical equations in react native. When typesetting fractions, baseline of the fraction must be aligned with the baseline of others. The baseline of fraction will vary based on size of numerator and denominator.
In yoga, we can set custom baseline function using `YogaNode.setBaselineFunction` method. If this method is exposed in `ReactShadowNode` class, it will be easy to create custom native UI modules which requires setting custom baseline.
Closes https://github.com/facebook/react-native/pull/15605
Differential Revision: D5686876
Pulled By: shergin
fbshipit-source-id: 34d797a7ea27d5c1b9f6b9c36e469cdca3883aec
Summary:
`react-native run-ios --device` should report 'No iOS devices connected.' in case when no devices attached. However due to logic bug when empty array is being treatened as a boolean value, cli reports odd message, saying `Could not find device with the name: "true".`
Disconnect all iOS devices from developer machine and run `react-native run-ios --device` in any project - console message should state that there are no devices available.
Closes https://github.com/facebook/react-native/pull/15603
Differential Revision: D5686891
Pulled By: shergin
fbshipit-source-id: 289461b6f920691ad39e940ecca6d85cf2a3573b
Summary:
If the project is in a folder with a path containing a space, the project won't build. This fixes this issue.
Closes https://github.com/facebook/react-native/pull/15608
Differential Revision: D5686861
Pulled By: shergin
fbshipit-source-id: 74ce5b4f0261c18070c1a48966aae24f1ca75492
Summary:
AnimatedImplementation.js is getting pretty hard to navigate and reason about so I split it up into different modules. Also took the opportunity to run prettier on that code and do some minor const/let refactorings. This doesn't change any logic, mostly just moves code around and add proper imports / exports.
This opens the door for further cleanup and flow type improvements but want to keep this already big PR as small as possible.
Discussion points:
- Should I use haste for this? Animated is pretty much a standalone module, it still uses a few haste imports but for new modules I used commonjs imports to avoid polluting the haste global namespace too much. The new modules are all internal to Animated and should not be imported externally.
- We're using `requestAnimationFrame` from fbjs instead of the one available globally in RN and browsers is there a reason for that?
- Should we even support web in this implementation? There is a standalone repo that exist for Animated web. Is this implementation of Animated web used internally at facebook?
- Probably still related to web, we used some weird Set polyfill is that still needed?
Notes:
There is a small regression for docs where the type of some classes that are exported like AnimatedValue show up as CallExpression instead if Class.
<img width="655" alt="screen shot 2017-08-14 at 3 19 18 am" src="https://user-images.githubusercontent.com/2677334/29261820-8f243036-809f-11e7-8bf0-0fe9f93939ca.png">
**Test plan**
Tested that all Animated related examples still work in RNTester on iOS and Android.
Tested that the doc is still working
Ran unit tests
Closes https://github.com/facebook/react-native/pull/15485
Differential Revision: D5679886
Pulled By: sahrens
fbshipit-source-id: d3e9b6987ab5ff2cd20108c3b9d860c7536be8a0
Summary:
> The property contentInset can change the maximum and minimum values of the content offset to allow scrolling outside of the scrollable area. Its type is UIEdgeInsets, which consists of 4 numbers: {top, left, bottom, right}. When you introduce an inset, you change the range of the content offset. For example, setting the content inset to have a value of 10 for its top value allows the content offset’s y value to reach -10. This introduces padding around the scrollable area.
( https://www.objc.io/issues/3-views/scroll-view/ )
See also: https://github.com/facebook/react-native/pull/15395
Reviewed By: mmmulani
Differential Revision: D5607192
fbshipit-source-id: 1acd6a84e2bcfefc6e82861cfbdfe6247d0e4264
Summary: Preparation before using that from elsewhere that's already typed.
Reviewed By: cpojer
Differential Revision: D5678616
fbshipit-source-id: 01ef6e688241e07ae9ce6aba27c6d4980f6a688b
Summary:
The `3.0.0` contained syntax errors for older versions of Node.
See also https://github.com/facebook/react-native/issues/15496
Reviewed By: cpojer
Differential Revision: D5677965
fbshipit-source-id: cae07fdce7e887c6fb1d6087791db8307f6f72f3
Summary:
== Before ==
- Aspect ratio would do its best to fit within it's parent constraints
- Aspect ratio would prioritize `alignItems: stretch` over other sizing properties.
== After ==
- Aspect ratio is allowed to make a node grow past its parent constraints. This matches many other aspects of flexbox where parent constraints are not treated as hard constraints but rather as suggestions.
- Aspect ratio only takes `alignItems: stretch` into account if no other size definition is defined. This matches the interaction of other properties with `alignItems: stretch`.
== Updating your code ==
**You probably don't need to do anything** but in case something does break in your product it should be as easy as adding `{width: '100%', height: '100%', flexShrink: 1}` to the style declaring the `aspectRatio`.
Reviewed By: gkassabli
Differential Revision: D5639187
fbshipit-source-id: 603e8fcc3373f0b7f2461da2dad1625ab59dcb19
Summary:
fix a typo on linux keyboark ( Use `ctrl` instead of `command` )
Closes https://github.com/facebook/react-native/pull/15565
Differential Revision: D5666988
Pulled By: TheSavior
fbshipit-source-id: 3e796f4e155c6c76ce1b7c0f2b473a8f64e1d0bd
Summary:
Currently, `onLoadStart` fires for a couple of cases:
1. toplevel page loads (e.g. initial page load, clicking links)
2. loading of pages within an iframe
The fact that `onLoadStart` fires for case (2) causes some problems. For example, it makes it difficult for the code that uses the WebView to know what URL the WebView is currently rendering. This is because the listener can't distinguish between the toplevel URL and the URL of an iframe. Additionally, this behavior is inconsistent with the behavior on iOS. On iOS, `onLoadStart` only fires for toplevel page loads.
To fix these issues, this change deletes the `doUpdateVisitedHistory` handler so that `onLoadStart` only fires for case (1).
**Test Plan**
Created a test page that has an iframe and loaded it in the WebView. Verified that `onLoadStart` only fires for toplevel page loads.
Adam Comella
Microsoft Corp.
Closes https://github.com/facebook/react-native/pull/15554
Differential Revision: D5665979
Pulled By: hramos
fbshipit-source-id: a52e473bc5691a6e180f45f0728e4ad89a7d354f
Summary:
Overview -
This PR resolves the issue described in #14606. This PR makes Text components take into account the includeFontPadding property when calculating their size.
Background -
Currently, on Android, when includeFontPadding is set to false, the React Text component does not adjust its height. This makes it difficult to lay out other components at a precise spacing relative to a Text component.
iOS calculates the height of a UILabel based on the font's descent + ascent.
Android lets you choose whether to calculate the height of a TextView based on the font's top + bottom (includeFontPadding=true) or ascent + descent (includeFontPadding=false).
In order for a text component to be the same size on iOS and Android (relative to the rest of the layout in points and dips), one should set includeFontPadding=false on Android - but the React Text component needs to take this property into account when sizing itself for this to work.
Please see this stack overflow post for a visual explanation of the difference between a font's ascent/descent and top/bottom - https://stackoverflow.com/questions/27631736/meaning-of-top-ascent-baseline-descent-bottom-and-leading-in-androids-font
Testing -
Please see the attached screenshots to see the height difference of a Text component with this PR when includeFontPadding is true vs false.
The font I am using has an ascent + descent = em-size so that the height of the Text component will be equal to the font-size for a single line of text. This is to clearly show the additional height that includeFontPadding=true adds to the Text component.
For Text components that are styled in the same way,
When includeFontPadding=true, height = ~29.714 dips
When includeFontPadding=false, height= 24 dips
<img width="342" alt="includefontpaddingtrue" src="https://user-images.githubusercontent.com/1437344/27299391-3eec9de0-54fa-11e7-81d5-d0aeb40e8e27.png">
<img width="346" alt="includefontpaddingfalse" src="https://user-images.githubusercontent.com/1437344/27299401-45c95248-54fa-11e7-98d7-17dd152d3cb8.png">
Closes https://github.com/facebook/react-native/pull/14609
Reviewed By: AaaChiuuu
Differential Revision: D5587602
Pulled By: achen1
fbshipit-source-id: 6d2f12ba72ec7462676645519cd27820279278eb
Summary:
It's probably better to link to main NDK page than downloads section.
Closes https://github.com/facebook/react-native/pull/13764
Differential Revision: D5661133
Pulled By: hramos
fbshipit-source-id: 18a54cada38bb894f2535e26b218818a854d699f
Summary:
When the AppState module is initialized, it subscribes to the `appStateDidChange` event and sends an async native method call to the AppState native module. There is a small race condition window where the native module can read the current app state as `uninitialized` before calling the JavaScript callback, and then be interrupted by the underlying mechanism to trigger the `appStateDidChange` event. If the `appStateDidChange` event is processed before the JavaScript callback, the resulting value of `AppState.currentState` will be invalid.
FixesMicrosoft/react-native-windows#1300
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
I simulated (over-exaggerated) the race condition by injecting "thread sleep" calls in the native method call and the native mechanism for updating the app state.
I then ran the AppStateExample in the RNTester and found that the current app state was set to `uninitialized`, as opposed to the expected value of `active`.
Once I made this JavaScript change, the over-exaggerated race condition no longer resulted in an invalid app state.
Closes https://github.com/facebook/react-native/pull/15499
Differential Revision: D5660620
Pulled By: hramos
fbshipit-source-id: 47c0dca75d37f677191c48f2148a72edd9cdd0e2
Summary:
On an iPhone running iOS 10.3.3, I'm not seeing the UIContentSizeCategoryDidChangeNotification fire when changing the system's font scale setting. Registering for the event on `nil` instead of on the application object fixes the issue.
**Test Plan**
Verified that text in an RN app changes size when you change the system's font scale setting (Settings -> General -> Accessibility -> Larger Text). By default, RN text is supposed to scale with the system setting.
Adam Comella
Microsoft Corp.
Closes https://github.com/facebook/react-native/pull/15522
Differential Revision: D5657449
Pulled By: shergin
fbshipit-source-id: d7a6a730c5cd6eb3833274a1f2029663ad3a7ad2
Summary:
This commit enables state WebViewState before triggering reload on WebView. This will (if defined) trigger the loading screen again.
On iOS the LoadingIndicator will be called whenever you reload the WebView. On Android this feature is missing (see #11013). This PR adds this behaviour.
Important: One might think that "onLoadStart" is the right area to add this code, but on Android onLoadStart will also trigger for sub-resources -> the loading screen will also appear when loading iframes on the same page. I expect thats why this was not added in first place.
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Closes https://github.com/facebook/react-native/pull/15538
Differential Revision: D5653257
Pulled By: hramos
fbshipit-source-id: 908b82ddaf2c34048bcb833bc07e03ab68d09467
Summary:
**Motivation**
On Apple TV, pressing the menu button destroys the native view that backs the `Modal` component, causing an app using this component to get into a broken state. This fix implements `onRequestClose` for tvOS to have the same behavior as it does for the Android back button.
**Test plan**
Manually tested this with the `ModalExample` in the `RNTester` app. See also the test code in issue #15313.
Closes https://github.com/facebook/react-native/pull/15341
Differential Revision: D5651035
Pulled By: shergin
fbshipit-source-id: 54bf66887bbe85940567e63e90b437ac4a8daf9a
Summary:
<!--
Thank you for sending the PR! We appreciate you spending the time to work on these changes.
Help us understand your motivation by explaining why you decided to make this change.
You can learn more about contributing to React Native here: http://facebook.github.io/react-native/docs/contributing.html
Happy contributing!
-->
(Write your motivation here.)
(Write your test plan here. If you changed any code, please provide us with clear instructions on how you verified your changes work. Bonus points for screenshots and videos!)
Closes https://github.com/facebook/react-native/pull/15515
Differential Revision: D5648285
Pulled By: hramos
fbshipit-source-id: fc1e50ffd18cc234771c5f40f92549e7e87cd28e
Summary:
Hopefully, this will save an hour of WTFs for the next reader.
Closes https://github.com/facebook/react-native/pull/14876
Differential Revision: D5385881
Pulled By: hramos
fbshipit-source-id: 56961e8a5eb3b6fae4f92c7fb9fea1e1147a3521
Summary:
We sometimes get large PRs that sit for a long time without being reviewed. In some cases, the PR touches too many files or lines, and the author would benefit from splitting the PR into smaller easier-to-review PRs. In this PR, we automatically flag such PRs using the existing Danger PR automation functionality.
This PR achieves the objective by doing the following:
* Add facebook-open-source-bot to `IssueCommands.txt`, allowing it to add labels and close issues/PRs via facebook-github-bot commands.
* Adds a `large-pr` command to facebook-github-bot. This command will add a short blurb, apply a new "Large PR" label, and close the PR.
* Updates the Dangerfile to check for PRs that touch over 600 lines and/or files. In such cases it will warn and then issue the large-pr command.
Additional changes:
* Tag core contributor PRs with the "Core Team" label. This is a slight change in policy, in that the label used to be applied after the fact (after the PR was escalated for review). It's more convenient to monitor labels, so we'll start checking for PRs tagged as such and ensure they get escalated as needed. cc skevy
Thanks to TheSavior for suggesting this change.
Testing against PR #10084 which touches over 600 lines:
```
$ cd danger
$ DANGER_GITHUB_API_TOKEN="e622517d9f1136ea8900""07c6373666312cdfaa69" npm run danger pr https://github.com/facebook/react-native/pull/10084
> @ danger /Users/hramos/git/react-native/danger
> node ./node_modules/.bin/danger "pr" "https://github.com/facebook/react-native/pull/10084"
{
fails: [],
warnings: [
{
message: "📋 Test Plan - <i>This PR appears to be missing a Test Plan.</i>"
},
{
message: "❗ Big PR - <i>This PR is extremely unlikely to get reviewed because it touches 613 lines.</i>"
}
],
messages: [],
markdowns: ["facebook-github-bot label Needs more information", "facebook-github-bot large-pr"]
}
```
Result: successfully flagged as being a large PR.
Closes https://github.com/facebook/react-native/pull/15519
Differential Revision: D5647085
Pulled By: hramos
fbshipit-source-id: dfa0f6580b9373085eba856de257a8d2452efbdc
Summary:
Thanks for submitting a PR! Please read these instructions carefully:
- [x ] Explain the **motivation** for making this change.
- [x ] Provide a **test plan** demonstrating that the code is solid.
- [x ] Match the **code formatting** of the rest of the codebase.
- [x ] Target the `master` branch, NOT a "stable" branch.
The section on changing ports is missing a key step for folks wishing to run on device
What existing problem does the pull request solve?
Changing packager port to something other than 8081 also requires the react native xcode project file to be updated in order to be able to build and run on device. The section on changing ports currently does not address this. See https://github.com/facebook/react-native/issues/10715#issuecomment-258082512.
Documentation update, no tests required.
A good test plan has the exact commands you ran and their output, provides screenshots or videos if the pull request changes UI or updates the website. See [What is a Test Plan?][1] to learn more.
If you have added code that should be tested, add tests.
Sign the [CLA][2], if you haven't already.
Small pull requests are much easier to review and more likely to get merged. Make sure the PR does only one thing, otherwise please split it.
Make sure all **tests pass** on both [Travis][3] and [Circle CI][4]. PRs that break tests are unlikely to be merged.
For more info, see the ["Pull Requests"][5] section of our "Contributing" guidelines.
[1]: https://medium.com/martinkonicek/what-is-a-test-plan-8bfc840ec171#.y9lcuqqi9
[2]: https://code.facebook.com/cla
[3]: https://travis-ci.org/facebook/react-native
[4]: http://circleci.com/gh/facebook/react-native
[5]: https://github.com/facebook/react-native/blob/master/CONTRIBUTING.md#pull-requests
Closes https://github.com/facebook/react-native/pull/14293
Differential Revision: D5647393
Pulled By: hramos
fbshipit-source-id: 040c3edea7d25c0efe560d5df30e86e1f26355bc
Summary:
This is a workaround for missing PDF url support in Android WebView, which is a general known issue: when tapping a PDF url within WebView, instead of doing nothing, we just let android default intent handle it (e.g. it will open Chrome to load it).
This is basically to trick `shouldOverrideUrlLoading()` to return true for the specific url. The drawback is that product code needs to provide the whitelist.
The proper fix would be to use PdfRenderer in that method, but it seems like it's only for API >= 21...
Differential Revision: D5619383
fbshipit-source-id: f86b930f970dab9a5f57999df69ce94b9508edc9
Ivan Ha
Alin Panaitiu
Peter Ruibal
Spencer Ahrens
Matt Oakes
Rafael Oleza
Lukas Wöhrl
Sreejith Krishnan R
Vladimir Kotikov
Gabriel Bull
Janic Duplessis
Valentin Shergin
Scott Wolchok
Jean Lauliac
Alexey Lang
Kathy Gray
Naman Goel
Andrew Y. Chen
Michał Gregorczyk
Emil Sjolander
S.M.A. Javadi
Alexander Mykolaichuk
Adam Comella
Héctor Ramos
Misha Greenberg
Marc Horowitz
Radek Czemerys
Eric Rozell
Andres Suarez
Ethan Davis
Caleb Meredith
Ben Nham
Eli White
DracoBlue
Douglas Lowder
Paco Estevez Garcia
Tomas Reimers
Petr Bela
George Jose
Kevin Gozali
