Summary:
Changelog: [Internal]
# Problem
We were recording mount child component calls with its arguments and the replaying them inside `finalizeUpdates`.
However we store the events in NSDictionary where `key` was index at which the child should be added.
Then in `finalizeUpdates` we iterated over this NSDictionary and added those views into our paper view.
`NSDictionary` is unordered, it isn't guaranteed what was first inserted into it, will be first iterated over.
# Solution
Use `NSMutableArray` instead which guarantees order.
Reviewed By: shergin
Differential Revision: D21685993
fbshipit-source-id: 3b933f05125130eef175d7a8a56f29012ee76bb3
Summary:
Changelog: [Internal]
If you look at implementation of hit testing in `RCTViewComponentView`
```
- (BOOL)pointInside:(CGPoint)point withEvent:(UIEvent *)event
{
if (UIEdgeInsetsEqualToEdgeInsets(self.hitTestEdgeInsets, UIEdgeInsetsZero)) {
return [super pointInside:point withEvent:event];
}
CGRect hitFrame = UIEdgeInsetsInsetRect(self.bounds, self.hitTestEdgeInsets);
return CGRectContainsPoint(hitFrame, point);
}
```
you will notice that we use `UIEdgeInsetsInsetRect` to calculate hitFrame. The input for this function is bounds and `hitTestEdgeInsets`. `hitTestEdgeInsets` is our hitSlop.
Look at documentation of `UIEdgeInsetsInsetRect`, it says "Adjusts a rectangle by the given edge insets.".
So if you give it a positive edge insets, it will make the rect smaller.
That's why we need to reverse values of hitSlop to negative before assigning it to `hitTestEdgeInsets`.
Paper does the same thing here https://github.com/facebook/react-native/blob/d0871d0a9a373e1d3ac35da46c85c0d0e793116d/React/Views/RCTViewManager.m#L304-L305
Reviewed By: mdvacca
Differential Revision: D21661894
fbshipit-source-id: c3dd6c55b68e4fdef8589ca8f0484e2837b4136c
Summary:
Changelog: [Internal]
# Problem
We call `_state->updateState` anytime safe area insets changes.
Once that is called, `_state` still holds old value, so when we check whether there is big enough of a difference, we are actually checking it against old value of state until `updateState` is called from MountingManager.
This causes `_state->updateState` to be called dozens of times (I measured 47, with this diff it fell to 22) when displaying a safe area view inside Modal.
# Solution
Create new ivar `_lastPaddingStateWasUpdatedWith` where we store last padding that was sent through `_state->updateState` and compare new inset with this value instead of last value stored in state.
Reviewed By: shergin
Differential Revision: D21596367
fbshipit-source-id: b9249b8ef444ea532ec8b86a15a32c733eb6f987
Summary:
Changelog: [Internal]
Using `empty()` vs `size() == 0` or `size() > 0`.
It is a more semantic way to check whether container is empty or not.
Reviewed By: JoshuaGross
Differential Revision: D21573183
fbshipit-source-id: b83283f687432a037941852114717a0f014e28db
Summary:
Changelog: [Internal]
ScrollView didn't support centerContent prop. The implementation is copied over from Paper.
I added an example of this prop in RNTester.
Reviewed By: JoshuaGross
Differential Revision: D21548270
fbshipit-source-id: 28f6c8d769c5a6bc1d111b33970a06b95c259132
Summary:
Changelog: [internal]
As part of recycle, we should delete state. This is a common pattern used in other components as well.
Reviewed By: shergin
Differential Revision: D21348782
fbshipit-source-id: a5dee2f4ccee9b19498db31dab1983d8879dca71
Summary:
In D20836890 we no longer set `_contentView.frame` inside `layoutSubviews`.
This doesn't work well with component views that set their `contentView` after `updateLayoutMetrics` is called. For Example legacy interop which sets its contentView in the `finalizeUpdates` method. Other component views that set their contentView during initialisation are fine.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D21302108
fbshipit-source-id: 5adcf489e4e650ac04b84c836a8f8d6f55a77267
Summary:
Changelog: [Internal]
When there is a change coming from javascript, avoid triggering `onChange ` and `onSelectionChange` events since JavaScript already has these changes. This is only necessary for multiline text.
For changes coming through view commands, do not increment `_mostRecentEventCount`.
Reviewed By: shergin
Differential Revision: D21255228
fbshipit-source-id: e972dab0eb3dd21f611ee5c71d755bab593ae9cc
Summary:
Changelog: [Internal]
In `onKeyPress` event, we were not returning `key` property. This diff adds `key` property to `onKeyPress` event and removes other, redundant properties from `onKeyPress` event.
The implementation has been translated from Paper.
Reviewed By: shergin
Differential Revision: D21250411
fbshipit-source-id: f1e31381667acb9dec02d0b33883df8f8f5b2a4b
Summary:
We need to break up the `uimanager` module in order to solve circular dependencies problem (which future diff would have otherwise).
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D20885163
fbshipit-source-id: 08eb1ba1d408fc0948e8d0da62380786a40973af
Summary:
Changelog: [Internal]
We don't use view command `setMostRecentEventCount`, let's get rid of it.
Reviewed By: JoshuaGross
Differential Revision: D21016600
fbshipit-source-id: 6491c063e9d6a89252300cb47c010b248e473f4b
Summary:
Changelog: [Internal]
Calling `_backedTextInputView.attributedText = attributedString` causes cursor to be moved to the end of text input.
This applies to both, `UITextField` and `UITextView`.
This is not desired as when JS sets a new text, we don't want the cursor to be moved to the end of text input.
JS has the option to use view commands if it wishes to move cursor somewhere.
Reviewed By: JoshuaGross
Differential Revision: D20836201
fbshipit-source-id: 9234e54cfbc5fc206f723626988e505275788aae
Summary:
Changelog: [Internal]
UIKit uses either `UITextField` or `UITextView` as its UIKit element for `<TextInput>`. `UITextField` is for single line entry, `UITextView` is for multiline entry.
There is a problem with order of events when user types a character.
In `UITextField` (single line text entry), typing a character first triggers `onChange` event and then `onSelectionChange`. JavaScript depends on this order of events because it uses `mostRecentEventCount` from this even to communicate to native that it is in sync with changes in native.
In `UITextView` (multi line text entry), typing a character first triggers `onSelectionChange` and then `onChange`. As JS depends on the correct order of events, this can cause issues. An example would be a TextInput which changes contents based as a result of `onSelectionChange`. Those changes would be ignored as native will throw them away because JavaScript doesn't have the newest version.
Reviewed By: JoshuaGross
Differential Revision: D20836195
fbshipit-source-id: fbae3b6c0d388fc059ca2541ae980073b8e5f6c7
Summary:
Changelog: [Internal]
Setting `_borderLayer.frame` inside `-[RCTViewComponentView layoutSubviews]` causes unwanted animation because it is not wrapped in `CATransaction`.
Moving it to `-[RCTViewComponentView updateLayoutMetrics]` which is called inside `CATransaction`.
Reviewed By: shergin
Differential Revision: D20836890
fbshipit-source-id: 2048a25fd2edb8109f6275c1186c0adae4b9f504
Summary:
Changelog: [Internal]
# There are three changes in this diff
## _stateRevision is replaced with a BOOL
`_stateRevision` was protecting against setting attributed string that is already visible to the user. Previously this was ok because the change was only coming from native, any changes from JS were ignored.
Imagine following scenario:
1. User taps key.
2. Update state is called on component initiated by native.
3. New state is created with incremented revision by one.
4. `_stateRevision` gets set to new state's revision + 1.
5. Now JS wants to change something because it just learnt that user tapped the key.
6. New state is created again with incremented revision by one.
7. Update state is called on the component, but the change isn't applied to the text view because `_state->getRevision()` will equal `_stateRevision`.
By having a BOOL instead of number, we very explicitly mark the region in which we don't want state changes to be applied to text view.
## Calling [_backedTextInputView setAttributedText] move cursor to the end of text input
This is prevented by storing what the current selection is and applying it after `[_backedTextInputView setAttributedText]` is called.
This was previously invisible because JS wasn't changing contents of `_backedTextInputView`.
## Storing of previously applied JS attributed string in state
This is the mechanism used to detect when value of text input changes come from JavaScript. JavaScript sends text input value changes through props and as children of TextInput.
We compare what previously was set from JavaScript to what is currently being send from JavaScript and if they differ, this change is communicated to the component.
Previously only first attributed string send from JavaScript was send to the component.
# Problem
If children are used to set text input's value, then there is a case in which we can't tell what source of truth should be.
Let's take following example
We have a text field that allows only 4 characters, again this is only a problem if those 4 characters come as children, not as value.
This is a controller text input.
1. User types 1234.
2. User types 5th character.
3. JavaScript updates TextInput, saying that the content should stay 1234.
4. In `TextInputShadowNode` `hasJSUpdatedAttributedString` will be set to false, because previous JS value is the same as current JS value.
Reviewed By: shergin
Differential Revision: D20587681
fbshipit-source-id: 1b8a2efabbfa0fc87cba210570142d162efe61e6
Summary:
Changelog: [Internal]
# Fabric
1. If `start` and `end` parameters in `setTextAndSelection` are -1, we don't move the cursor. Previously the cursor would be moved to beginning of text input.
2. In view commands, do not validate `eventCount`. It is passed in as undefined from JS because Fabric's text input doesn't use `eventCount`.
# Paper
1. If `start` and `end` parameters in `setTextAndSelection` are -1, we don't move the cursor. Previously the cursor would be moved to beginning of text input.
Reviewed By: shergin
Differential Revision: D20538290
fbshipit-source-id: c7aeddc25f58697254474058ce901df958321f7c
Summary:
The new prop on RCTParagraphComponentView is meant to be used only by external introspection tools, not the RN core.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: nscoding
Differential Revision: D20523078
fbshipit-source-id: 4c457d070fd2c172d681c5aa7f731d2d52bba291
Summary:
Changelog: [Internal]
# Problem
SafeAreaView is getting reused, its previous `safeAreaInsets` is top: 44, bottom: 34 so `safeAreaInsetsDidChange` doesn't get called because it doesn't change. Therefore state gets never updates because `safeAreaInsetsDidChange` is never called.
# Solution
Update state whenever a new state is assigned.
Reviewed By: shergin
Differential Revision: D20444198
fbshipit-source-id: 75d1458450c70d74647df4962ddad88d5f6a38d2
Summary:
Now we have `zIndex` feature implemented in the Core, we don't need to take `view.layer.zIndex` into account when we do hit-testing, so now we don't need to sort *all subviews* on *all levels of hierarchy* every time we process touch down event.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20425987
fbshipit-source-id: 025bd968ae948b9b0a4188845efc0de950fb5cdf
Summary:
Now we have `zIndex` feature implemented in the Core, we don't need to have it implemented on the mounting layer.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20432156
fbshipit-source-id: f77b96919bab21b6628472b9fe58c5f4e3233318
Summary:
Changelog: [Internal]
# Problem
`scrollView.state.contentOffset` was not in sync with actual `contentOffset` in case `contentOffset` is changed programatically.
# Solution
Add a flag `_isUserTriggeredScrolling` that indicates whether the current scroll is triggered by user or not. In case it isn't, update state.
Reviewed By: shergin
Differential Revision: D20098161
fbshipit-source-id: 021d916e7a45a24095a47bb8f84d1102226b672a
Summary:
Investigating a crash, I spend half of an hour staring at `__bridge`, `__bridge_retained`, `CFRelease` and etc trying to understand is there a bug or not. Even if I think there was no bug there, it should not be this way. We have a nice abstraction around that madness we should use to make the code obvious.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20260917
fbshipit-source-id: 2b511ebf46a78950c4720e749099134aed1dd890
Summary:
We don't know why exactly it happens but (seemingly) sometime Paragraph component receives an empty State object. This causes a crash because of unchecked access to an instance variable.
This diff introduces an assert in DEBUG mode and the check for prod.
Why is this happens? Hard to say, probably `layout()` method (which updates `State`) is not being called on ShadowNode. Why? One explanation can be that Yoga skips some subtree during layout because it starts from "visibility: hidden" component. We need to investigate it more and figure out what exactly going on and what should we need to improve besides the check. That's why we have an assertion.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D20260512
fbshipit-source-id: 4772855f41c6694be2ed6c0a19da40332d2bb8db
Summary:
Changelog: [Internal]
# Problem
Calling `UIManager::updateState` does not increment state revision because it calls `ConcreteComponentDescriptor::createState` which creates new state with state revision 1.
# How did this propagate?
This error propagated itself in TextInput when trying to input a value, you would be only allowed to type in 1 character.
Reviewed By: JoshuaGross
Differential Revision: D20072844
fbshipit-source-id: 37b8173307e1d91d6e9c41b5ff2e185dde31cc38
Summary:
Changelog: [Internal]
If `layer.mask` was set and the view got reused without having a different `layer.mask`, this value would persist between reuses.
I also added a call to `super finalizeUpdates` as it is best practice to call super, the parent class right now doesn't do anything but in the future we might add there some default logic.
Reviewed By: shergin
Differential Revision: D20030174
fbshipit-source-id: c90be3f4e9a8f3814000f177a3d50061f5aa120c
Summary:
Changelog: [Internal]
When I was originally implementing this view command (D19471025), I misunderstood the desired behaviour.
The text parameter isn't meant to change text in the specified `select` but it is supposed to override text of entire text input.
Reviewed By: shergin
Differential Revision: D19793484
fbshipit-source-id: 64ba36ddfa27ac5a0adf48495cb4e985a429e005
Summary:
Starting on iOS 13, a View Controller presented modally will have a "bottom sheet" style unless it's explicitly presented full screen.
Before this, modals on iOS were only being dismissed programatically by setting `visible={false}`. However, now that the dismissal can happen on the OS side, we need a callback to be able to update the state.
This PR reuses the `onRequestClose` prop already available for tvOS and Android, and makes it work on iOS for this use case.
Should fix https://github.com/facebook/react-native/issues/26892
## Changelog
[iOS] [Added] - Add support for onRequestClose prop to Modal on iOS 13+
Pull Request resolved: https://github.com/facebook/react-native/pull/27618
Test Plan:
I tested this using the RNTester app with the Modal example:
1. Select any presentation style other than the full screen ones
2. Tap Present and the modal is presented
3. Swipe down on the presented modal until dismissed
4. Tap Present again and a second modal should be presented
Differential Revision: D19235758
Pulled By: shergin
fbshipit-source-id: c0f1d946c77ce8d1baab209eaef7eb64697851df
Summary:
Changelog: [Internal]
I went over the ownership model in this call `_state->getData().getImageRequest().getImageInstrumentation().didSetImage();`
1. `getData()` returns reference to an object that is copied to state.
2. `getImageRequest()` returns a reference to an object that is strongly owned by `ImageState`.
3. `getImageInstrumentation()` returns a reference to an object that is strongly owned by `ImageRequest`.
I don't think any of these can cause a crash, however `_state` can be nullptr.
Reviewed By: shergin
Differential Revision: D19599258
fbshipit-source-id: 317f03cd9a53d83f64ccbb9f9abfce10fcc1fae5
Summary:
Previously, State revision number was implemented manually as part of the StateData. Now we have it as a built-in concept in State, so we can rely on that.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19467161
fbshipit-source-id: cac907265090730cdb89207aad2b52141cda5dc6
Summary:
There is no guarantee that the pointer is not null, so we have to check for it. And particularly, the `instrumentation` pointer is null when the `ImageSource`'s type is `invalid`.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: sammy-SC
Differential Revision: D19482208
fbshipit-source-id: e1eab02841b1594136388e51d9d5b5c881ab9366
Summary:
Add Native Commands handlers to TextInput.
It is intentionally done in a way so that it can be easily swapped for codegened implementation once we properly type TextInput.
We also add native commands to view managers for backwards compatibility.
Changelog: [Internal]
Reviewed By: TheSavior, shergin
Differential Revision: D19412026
fbshipit-source-id: 8dc64275cf1da599b1bd5992a41035d51dd4148f
Summary:
Re-landing the reverted change:
This removes all callsites that rely on runtime checks to detect the target deployment version. We no longer need to check for iOS 10+ in a few places. Note: for this to compile, the hosting app needs to target iOS 10.0+.
Changelog: [iOS] [Deprecated] - Deprecate iOS 9
Reviewed By: sammy-SC
Differential Revision: D19411136
fbshipit-source-id: ec0a957dc57819f0ee7d138c858209cabe3e5102
Summary:
This removes all callsites that rely on runtime checks to detect the target deployment version. We no longer need to check for iOS 10+ in a few places. Note: for this to compile, the hosting app needs to target iOS 10.0+.
Changelog: [iOS] [Deprecated] - Deprecate iOS 9
Reviewed By: hramos
Differential Revision: D19271321
fbshipit-source-id: 424ad7e2161261d148cb436cc20b4c531a4ba5b7
Summary:
In codegen we generate structs that represents events. These structs are later dispatched by generated `EventEmitter`.
They had unpleasant naming, for example `SliderOnValueChangeStruct`. This diff changes the code generated so it becomes `SliderEventEmitter::OnValueChange`, this better expresses the relationship of the two classes.
Changelog: [Internal]
Motivation: Better express relationship between EventEmitter and classes that represent events.
Reviewed By: rickhanlonii, shergin
Differential Revision: D19373850
fbshipit-source-id: a5eea085013dbc119169e2b06ba9f9fe44c7fcd9
Summary:
This is the partial implementation of Fabric-compatible <TextInput> component on iOS. All features are supported besides those:
* `focus()`, `blur()`, `clear()` imperative calls;
* Controlled TextInput as the whole feature in general;
* Controlling selection from JavaScript side;
* `autoFocus` prop;
* KeyboardAccessoryView.
Changelog: [Internal]
Reviewed By: JoshuaGross
Differential Revision: D17400907
fbshipit-source-id: 0ccd0e0923293e5f504d5fae7b7ba9f048f7d259
Summary:
RCTTextInputUtils contains a bunch of conventions and convenience functions that we use in TextInput.
Changelog: [Internal] Fabric-specific internal change.
Reviewed By: JoshuaGross
Differential Revision: D19226658
fbshipit-source-id: df72dcdc052b96b6daef2cc0839235761005d914
Summary:
ImageState if created with default constructor is created with `imageRequest_` being nullptr.
Calling `getObserverCoordinator()` on it was causing a crash.
We create initial state data with `imageRequest` populated.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D19332711
fbshipit-source-id: 0266222551dbfb10b3f86e72a43d5306650fd09b
Samuel Susla
Valentin Shergin
Jorge Bernal
Kevin Gozali
Jiawei Lv
