Remove defaultProps from DrawerLayoutAndroid #31647
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Hi @Setito! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
Base commit: b77948e |
Base commit: b77948e |
facebook-github-bot
added
the
CLA Signed
Contributor
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
lunaleaps
requested changes
Jun 8, 2021
Contributor
lunaleaps
left a comment
lunaleaps
left a comment
There was a problem hiding this comment.
Thank you for working on this!
We'll still want to use 'white' as the default drawerBackgroundColor but not leverage React's defaultProps. For example on line 192, we're still reading the value of drawerBackgroundColor, if that value is null/undefined, we'd like to use the default of 'white'.
If you run the jest tests (I believe through yarn test) then I think the DrawerLayoutAndroid-test will fail currently.
pedrocardoz0
left a comment
pedrocardoz0
left a comment
There was a problem hiding this comment.
At line 177 try to use something like this:
const {
onDrawerStateChanged,
drawerBackgroundColor="white"
...props
} = this.props;
|
Summary: With the conclusion of D27944688 (facebook@925af8d), this backs out the experiment introduced via D27987619 (facebook@f598dd0). Changelog: [Internal] Reviewed By: kacieb Differential Revision: D28799741 fbshipit-source-id: 607ee85db26326e13dd8ddb52f5aebb732e9a354
Summary: jcenter is read-only now, and newer versions of dependencies will be published to either MavenCentral or Jitpack. This PR removes jcenter to avoid future issues, then uses MavenCentral and Jitpack as replacement. Current flipper depends on Stetho version that is not available on MavenCentral, so had to exclude and bump the version. Both Gradle and Buck successfully download all the dependencies. ## Changelog [Android] [Changed] - Remove jcenter Pull Request resolved: facebook#31609 Test Plan: rn-tester builds and runs as expected. Reviewed By: mdvacca Differential Revision: D28802444 Pulled By: ShikaSD fbshipit-source-id: 043ef079d0cda77a1f8dd732678452ed712741a4
Summary: This hack should not be necessary. It should be fixed at Differ or LayoutAnimations level if there are existing issues there. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D28810021 fbshipit-source-id: 98b8d2ae9991ad527a3b3e90943d75063b2a4496
Summary: Virtual views that are flattened and don't "FormsView" on-screen should not be preallocated. Changelog: [Internal] Reviewed By: mdvacca Differential Revision: D28811419 fbshipit-source-id: 949dcbf4cf3791355c58af785603b35fa50f3f02
Summary: Since moving Switch to a function component it is no longer possible to get the native switch ref. This adds forwardRef so it is possible again. ## Changelog [General] [Fixed] - Fix Switch ref forwarding Pull Request resolved: facebook#31629 Test Plan: Tested the change in an app using react-native-gesture-handler, which tries to set a ref on Switch. Also made sure flow still passes. Reviewed By: TheSavior Differential Revision: D28839605 Pulled By: lunaleaps fbshipit-source-id: 1fee86145caeabb60c0010bb9062dddca419f7ca
Summary: Homebrew on M1 installs executable binaries in **/opt/homebrew/bin** (See https://brew.sh/2021/02/05/homebrew-3.0.0/), and FBReactNativeSpec.build is failing because it couldn't find node. This PR changes find-node.sh script to add /opt/homebrew/bin into $PATH. The way **react.gradle** trying to execute node is not using user environment variables, but system defaults, so it couldn't find it. I removed node execution, and hard coded cli path in parity with iOS https://github.com/facebook/react-native/blob/d1ab03235cb4b93304150878d2b9057ab45bba77/scripts/react-native-xcode.sh#L106 Fixes facebook#31621 facebook#31592 ## Changelog [General] [Changed] - find-node.sh supports Homebrew on M1 Pull Request resolved: facebook#31622 Test Plan: On M1, create a RN project and it'll fail to build iOS app. Apply the patch, and build will succeed. Reviewed By: ShikaSD Differential Revision: D28808206 Pulled By: hramos fbshipit-source-id: 8b313b6685462a15e67d99c61a0202d17fece1ec
Summary: The RuntimeExecutor that Fabric gets from the bridge doesn't call JSIExecutor::flush(). In the legacy NativeModule system, we're supposed to flush the queue of NativeModule calls after every call into JavaScript. The lack of this flushing means that we execute NativeModule calls less frequently with Fabric enabled, and TurboModules disabled. It also means that [the microtask checkpoints we placed inside JSIExecutor::flush()](https://www.internalfb.com/code/fbsource/[62f69606ae81530f7d6f0cba8466ac604934c901]/xplat/js/react-native-github/ReactCommon/jsiexecutor/jsireact/JSIExecutor.cpp?lines=427%2C445) won't be executed as frequently, with Fabric enabled. Changelog: [Android][Fixed] - Flush NativeModule calls with Fabric on Android, on every Native -> JS call. Reviewed By: JoshuaGross, mdvacca Differential Revision: D28620982 fbshipit-source-id: ae4d1c16c62b6d4a5089e63104ad97f4ed44c440
…ok#31597) Summary: Bumped react-native-community/cli to v6 to update metro to 0.66 to fix fast-refresh issues Also updated the manual test e2e script for easier testing. (using npm install would create a package-lock.json and conflict with yarn.lock) ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [GENERAL] [UPDATE] - updated react-native-community/cli to v6 (hence updating metro to 0.66) Pull Request resolved: facebook#31597 Test Plan: I've tested fast-refresh works with / without hermes Reviewed By: TheSavior Differential Revision: D28852660 Pulled By: yungsters fbshipit-source-id: af338e4dd1d52c62949d71f42773963d89bca9db
Summary: While it is possible in the React Native implementation for Android to provide a custom configuration for HTTP requests, the iOS implementation does not allow for the same customization. As the NSURLSession used for HTTP requests on iOS is configured internally, one may for instance not supply an ephemeral configuration for HTTP requests. Other concerns related to the given problem have been addressed in the community: react-native-community/discussions-and-proposals#166. I did make a PR with an RFC in the community repo, but after some discussion in the said repo, I figured I might as well make a PR with a suggestion :) ## Changelog [iOS] [Added] - Allow for configuring the NSURLSessionConfiguration Implement a C function `RCTSetCustomNSURLSessionConfigurationProvider` which gives the app programmer the ability to provide a block which provides an NSURLSessionConfiguration that will be used for all HTTP requests instead of the default configuration. The provided block will be called when the session configuration is needed. Pull Request resolved: facebook#27701 Test Plan: Unsure if this can be tested in any other way than uncommenting the example code in `RNTester/RNTester/AppDelegate.mm`. Reviewed By: yungsters Differential Revision: D28680384 Pulled By: JoshuaGross fbshipit-source-id: ae24399955581a1cc9f4202f0f6f497bfe067a5c
Summary: Sets up an injection mechanism for experimenting in production with an alternate implementation of `createAnimatedComponent`. This will be used to implement and refine a new `createAnimatedComponent` that is compatible with concurrent rendering. Changelog: [Internal] Reviewed By: lunaleaps Differential Revision: D28799739 fbshipit-source-id: 46ba2dd6137f7bf73ce8a659698533ed3985516f
Summary: Creates an experimental stub implementation of `createAnimatedComponent` to be implemented in future commits. Changelog: [Internal] Reviewed By: lunaleaps, kacieb Differential Revision: D28799738 fbshipit-source-id: dc3fbee557db353de6807bd87561f8f372d7cab5
Summary: Minor reorganization of `AnimatedGratuitousApp` to be a separate top-level directory in `examples/`, like `Animated`. Changelog: [Internal] Reviewed By: kacieb Differential Revision: D28799737 fbshipit-source-id: b0329e420d6eae912e91d0d74b68ac299c9bd9f3
Summary: Minor changes to the `Animated` examples index module to make the static typing a bit more intuitive. Changelog: [Internal] Reviewed By: kacieb Differential Revision: D28799742 fbshipit-source-id: f798631081538e79fc58377105db4e47b9728843
Summary: Removes an extra amount of 10dp margin on the top of each example. Changelog: [General][Fixed] - Remove excess spacing in RNTester examples. Reviewed By: kacieb Differential Revision: D28799736 fbshipit-source-id: 40c8db203f8119359ccc8c40fc0a6424de5afc5e
…e heap based array Summary: Don't allocate large arrays on stack when copying native pointers, use heap based array. Today the code copies the native pointers on the stack, since it may be too big, lets make sure to use heap based allocating using std::vector. This array is afterwards converted into a reversed map from index to pointer, so it is heap based anyhow. Changelog: [Internal] Don't allocate large arrays on stack when copying native pointers, use heap based array Reviewed By: Andrey-Mishanin Differential Revision: D28747213 fbshipit-source-id: da69b4b2d0960fdade9f07f44654b30d6dacc43a
…1479) Summary: A few places in RNTester where using hard coded color values, which meant the UI looks broken in dark themes. The area behind the bookmark button was using a solid color png file, which I've replaced with a color from the theme object. ## Changelog [Internal] [Fixed] - Fix a couple of places RNTester is using non-theme values Pull Request resolved: facebook#31479 Test Plan: Verified in RNTester in react-native-windows in light+dark theme. Reviewed By: TheSavior Differential Revision: D28290192 Pulled By: rozele fbshipit-source-id: 78192200ac2dc8629759c10f8e8b3ec2f6699acd
Summary: Gradle has been showing below warning for a while, and this PR fixes the warning using maven-publish plugin, thus taking us one step closer to Gradle 7.x. > The maven plugin has been deprecated. This is scheduled to be removed in Gradle 7.0. Please use the maven-publish plugin instead. Consult the upgrading guide for further information: https://docs.gradle.org/6.9/userguide/upgrading_version_5.html#legacy_publication_system_is_deprecated_and_replaced_with_the_publish_plugins Configured maven-publish plugin according to https://developer.android.com/studio/build/maven-publish-plugin, also added **installArchives** task for backwards compatibility. ## Changelog [Internal] [Changed] - use maven-publish plugin to build and publish Android artifact Pull Request resolved: facebook#31611 Test Plan: ./gradlew :ReactAndroid:installArchives will create **android** directory for local maven repository with **react-native** package. Reviewed By: yungsters Differential Revision: D28802435 Pulled By: ShikaSD fbshipit-source-id: 7bc7650a700e1a61213c5ec238bcb24fdca954db
Summary: Bump buildToolsVersion to 30.0.2, default version of Android Gradle Plugin 4.2.0. Fixes parity with facebook#31593 ## Changelog [Android] [Changed] - Bump buildToolsVersion to 30.0.2, Pull Request resolved: facebook#31627 Test Plan: Newly created projects will use build tools 30.0.2 to build dependencies. Reviewed By: yungsters Differential Revision: D28833598 Pulled By: ShikaSD fbshipit-source-id: 009472d27ea7103bdc7e5a6a941ab529d982f2da
…y using the platform roles Summary: Several accessibilityRole strings are hardcoded to only English on Android. Swap them to just use the platform constants. This way, TalkBack will handle translations. This change swaps roles "search", "button", and "togglebutton" roles to use the platform description. Changelog: [Android][Changed] Localize "search", "button", and "togglebutton" accessibility roles by using the platform roles Reviewed By: lunaleaps Differential Revision: D28077246 fbshipit-source-id: 5b88a6fd7e78b3426506f253b823ecca0608c4bc
Summary: Verifies interaction between PanResponder and ScrollView with JSResponderHandler. Also showcases how to create a swipeable card with scrollable content. Changelog: [Internal] Reviewed By: JoshuaGross Differential Revision: D28093313 fbshipit-source-id: 8ffbe734119912326c471412f4f5e096f64e87cc
Summary: Root nodes doesn't have a ComponentDescriptor that can be looked up via this mechanism, and we probably shouldn't be animating Root nodes anyway (?). This is a debug-only assert but could be causing issues in production. The fix is simple - just don't animate any changes to a root node. Changelog: [Internal] Reviewed By: Nick177 Differential Revision: D28856396 fbshipit-source-id: 43fa0aa723b03b031fee22e0563eb63cc86239b3
Summary: Pull Request resolved: facebook#31656 CircleCI's Windows executor currently ships with a pre-LTS release of Node v12, breaking our Windows jobs ([example](https://app.circleci.com/pipelines/github/facebook/react-native/9280/workflows/21e6e59c-d853-47a1-af62-1368c8ce10ce/jobs/203983)) following facebook#30637, ultimately due to jestjs/jest#10685 dropping support for non-LTS versions in the Node v12 release line. Luckily, the Windows executor [does ship with nvm](circleci/circleci-docs#3733) so we can use that to install a desired Node version. Rather than just pinning a later v12 release that is LTS, we pin a v14 release that is currently the most recent LTS version. NOTE: The nvm on CircleCI is https://github.com/coreybutler/nvm-windows, not https://github.com/nvm-sh/nvm, and the two aren't interchangeable. [nvm-windows has no functionality to install the latest version of a release line](coreybutler/nvm-windows#156) so we're forced to specify an exact version, which will need to be bumped manually in the future. This isn't great, but IMO it's no worse than the current situation, where we use whichever stale version of Node happens to be bundled with the Windows CircleCI executor. Changelog: [Internal] Reviewed By: GijsWeterings Differential Revision: D28896581 fbshipit-source-id: a412376cf36054de49efa49866fe60dd964567c5
Summary: Changelog: [internal] This is a life cycle issue where LayoutManager outlives the runtime. To fix this, we need to destroy `_accessibilityProvider` before the runtime. The way to do it is to destroy it inside `prepareForReuse` which is guaranteed to be called before runtime is destroyed. Reviewed By: JoshuaGross Differential Revision: D28898257 fbshipit-source-id: 9d2c0b9cebd9889caa4328f9ee7f005928bbf55a
Summary: setJSResponder/clearJSResponder have been in use in prod for a while and are stable. Ship them in code. Changelog: [Internal] Reviewed By: sammy-SC Differential Revision: D28889894 fbshipit-source-id: 1c42526cd890d528062eeb50761fc49cc6109d76
Summary: Changelog: [internal] Add a feature flag to enable yielding in RuntimeScheduler Reviewed By: JoshuaGross Differential Revision: D28903226 fbshipit-source-id: c361ca144a2d531e8aa671bc8875bce075e13a2c
Summary: Changelog: [General][Changed] Convert KeyboardAvoidingView to use export default Reviewed By: TheSavior Differential Revision: D28884111 fbshipit-source-id: e2d144505536362b405b833c9ebad530f6dcacb5
Summary: This fixes how ES modules are handled in Jest tests in the react-native codebase. They caused some problems before because `import` statements weren't inlined as `require` calls were, so there were some errors in tests when migrating from CommonJS to ESM. This changes the transform that Jest uses to inline import statements the same way, so we can migrate everything without issues in tests. Changelog: [Internal] Reviewed By: kacieb Differential Revision: D28899692 fbshipit-source-id: 027690f57ca3b5613c261a1089c0635af76662b2
Summary: The RN OSS release process includes manual testing that a new template app can be started under various platforms, JS engines, etc. This should ideally be automated, to help reduce wasted engineer-time, and to allow reliably increasing release velocity. `react-native-windows` does already have tests to create and build template projects across our matrix, but they do not do any runtime validation on the newly created app. Adding a `testID` to the new app screen header gives us something to search for in black-box testing to validate that the app started successfully. This should help catch cases where a sample project in repo has changes not reflected in a newly created template app. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [Internal] [Added] - Add testID to NewAppScreen Header Component Pull Request resolved: facebook#31652 Test Plan: Did not manually validate the change, though did check that `ImageBackground` forwards props to `Image`, and that `Image` is aware of `testID` and will forward to the native component. Reviewed By: kacieb Differential Revision: D28907197 Pulled By: p-sun fbshipit-source-id: db3974294afba25878383f1955cad37b69d95da3
…x length
Summary:
When maxLength is defined in <TextInput>, if the last character at max length is an emoji, the content of the input is cleared:
{F620865178} {F620865237}
Related Github issues:
facebook#10929
facebook#10964
## Root cause:
When NSString was created, unicode characters were 16-bit long, so Objective-C considers every unicode character as 16-bit. However, unicode was later extended to more than 16bit, for example, emojis, which causes NSString substring method cuts off at the wrong position.
Example:
```
NSString *s = @"abc{emoji:1f601}";
NSInteger len = s.length; //length is 5 (as {emoji:1f601} occupies two 16-bit characters)
NSString *s3 = [s substringToIndex: 3]; //s3 is "abc"
NSString *s4 = [s substringToIndex: 4]; //s4 is null!
```
If string s, "abc{emoji:1f601}", is entered in <TextInput>, which has max length 4, it will truncate the string to the first 4 characters, "cutting" the emoji in half which causes encoding error and returns null. The text input is cleared.
## Solution:
If the character at max length is longer than 16-bit, truncate the character BEFORE it instead. In the previous example, truncate till index 3 instead of 4. The end result will be "abc" and the emoji is dropped.
## Changelog:
[iOS] [Fixed] - <TextInput> content is reset when emoji is entered at the max length
Reviewed By: p-sun
Differential Revision: D28821909
fbshipit-source-id: 4720d864970b554160ed5388f65b352ce95a6199
Summary: Remove `defaultProps` from `Picker` of components, replace it with destructuring assignment. ## Changelog <!-- Help reviewers and the release process by writing your own changelog entry. For an example, see: https://github.com/facebook/react-native/wiki/Changelog --> [JavaScript] [Changed] - Remove defaultProps from picker close facebook#31603 Pull Request resolved: facebook#31644 Test Plan: all test suite and CI passes. Reviewed By: TheSavior Differential Revision: D28886320 Pulled By: lunaleaps fbshipit-source-id: d88a922dffeebe2bce019250d460b5e43a0af562
Summary: This issue fixes facebook#25696 fixes facebook#28854 fixes facebook#26193 Since Android API 28 it is possible to specify fontWeight with numerical values ranging from 100 to 900 This pr uses the new Typeface.create() method available on Android API 28+ to set font weight value ranging from 100 to 900, while still keeping existing functionalities (custom fonts, bold/italic and other styles). https://developer.android.com/reference/android/graphics/Typeface#create(android.graphics.Typeface,%20int,%20boolean) ## Changelog [Android] [Fixed] - Fix font weight numeric values Pull Request resolved: facebook#29117 Test Plan: Works in all scenarios. **<details><summary>CLICK TO OPEN TESTS RESULTS</summary>** <p> | **BEFORE** | **AFTER** | |:-------------------------:|:-------------------------:| | <img src="https://user-images.githubusercontent.com/24992535/84420949-1daa0e80-ac1b-11ea-9a2e-eaac03dc4533.png" width="300" height="" />| <img src="https://user-images.githubusercontent.com/24992535/84490766-edf31900-aca3-11ea-90d8-7c52d2e2be59.png" width="300" height="" /> | | **AFTER** | **AFTER** | |:-------------------------:|:-------------------------:| | <img src="https://user-images.githubusercontent.com/24992535/84490768-ee8baf80-aca3-11ea-8d3e-937d87b3c56a.png" width="300" height="" />| <img src="https://user-images.githubusercontent.com/24992535/84490769-ef244600-aca3-11ea-9dec-5eb70358834b.png" width="300" height="" /> | | **AFTER** | |:-------------------------:| | <img src="https://user-images.githubusercontent.com/24992535/84490772-f0557300-aca3-11ea-851a-5befc900192c.png" width="300" height="" />| </p> </details> Reviewed By: lunaleaps Differential Revision: D28917328 Pulled By: yungsters fbshipit-source-id: 8b84e855b3a8b87960cb79b9237d452b26974c36
Author
|
Thank you @pedrocardoz0 for reaching out and giving advice. |
Contributor
|
Closing this as I think #32162 addresses this issue now |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
20 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
Issue #31647. Removing the property defaultProps from the component DrawerLayoutAndroid.android.js
Changelog
[Android] [Removed] - Remove defaultProps from the DrawerLayoutAndroid component.
Test Plan