Skip to content

removed default props for drawerBackgroundColor and fixed lint issues#32162

Closed
megs-p wants to merge 3 commits intofacebook:mainfrom
megs-p:fix/removeDefaultPropsDrawerLayoutAndroid
Closed

removed default props for drawerBackgroundColor and fixed lint issues#32162
megs-p wants to merge 3 commits intofacebook:mainfrom
megs-p:fix/removeDefaultPropsDrawerLayoutAndroid

Conversation

@megs-p
Copy link
Contributor

@megs-p megs-p commented Sep 7, 2021

Summary

Removed the deaultProps in the DrawerLayoutAndroid file and replaced it with a default value in case props are undefined.

Changelog

[General] [Changed] - Remove defaultProps from the DrawerLayoutAndroid Component.
@lunaleaps this is the fix for issue #31606

Test Plan

Ran test suite

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 7, 2021
@megs-p
Copy link
Contributor Author

megs-p commented Sep 7, 2021

@cortinico i have not pushed the .bat files in this PR as well. Thanks for explaining what was happening.

@analysis-bot
Copy link

analysis-bot commented Sep 7, 2021 •
edited
Loading

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: a950634

@analysis-bot
Copy link

analysis-bot commented Sep 7, 2021 •
edited
Loading

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 8,957,335 -13
android hermes armeabi-v7a 8,490,628 +1
android hermes x86 9,377,554 -13
android hermes x86_64 9,342,875 -12
android jsc arm64-v8a 10,639,601 +9
android jsc armeabi-v7a 9,560,403 +13
android jsc x86 10,654,101 +11
android jsc x86_64 11,263,105 +14

Base commit: a950634

megs-p
megs-p commented Sep 7, 2021
View reviewed changes
Libraries/Components/DrawerAndroid/DrawerLayoutAndroid.android.js Outdated
* );
* ```
*/
drawerBackgroundColor?: ColorValue,
Copy link
Contributor Author

@megs-p megs-p Sep 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lunaleaps adding drawerBackgroundColor as an optional Prop here.

Copy link
Contributor

@lunaleaps lunaleaps Sep 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Let's make this drawerBackgroundColor?: ?ColorValue in case people spread an object with drawerBackgroundColor: null

Copy link
Contributor Author

@megs-p megs-p Sep 7, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure @lunaleaps will push a commit with that change so it covers all edge cases!

@megs-p megs-p mentioned this pull request Sep 7, 2021
Closed
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 7, 2021

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

@lunaleaps lunaleaps mentioned this pull request Sep 7, 2021
Closed
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 8, 2021

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

@facebook-github-bot facebook-github-bot added the Merged This PR has been merged. label Sep 8, 2021
@facebook-github-bot
Copy link
Contributor

facebook-github-bot commented Sep 8, 2021

@cortinico merged this pull request in b998e32.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lunaleaps lunaleaps lunaleaps left review comments

@cortinico cortinico cortinico approved these changes

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@megs-p @analysis-bot @facebook-github-bot @lunaleaps @cortinico