Expose additional private modules by brentvatne · Pull Request #3308 · facebook/react-native

brentvatne · 2015-10-09T16:20:14Z

TextInputState as TextInput.State
Touchable
flattenStyle as StyleSheet.flatten
ReactNativeART as ART

Original discussion in #1821

vjeux · 2015-10-09T18:25:27Z

facebook-github-bot · 2015-10-09T18:26:26Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/182740555392881/int_phab to review.

vjeux · 2015-10-09T20:54:36Z

website/server/extractDocs.js

mind doing a different pull request for this file, it only lives in open source

brentvatne · 2015-10-09T21:45:53Z

@vjeux - amended this PR to exclude the website change

vjeux · 2015-10-09T21:47:14Z

@facebook-github-bot shipit

facebook-github-bot · 2015-10-09T21:47:34Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/182740555392881/int_phab to review.

vjeux · 2015-10-09T22:12:02Z

There's a flow error

1 Flow error detected in root "Libraries/FBReactKit/js"!
=====Flow error #0====
Libraries/FBReactKit/js/react-native-github/Libraries/StyleSheet/StyleSheet.js line 74 col 1-18: assignment of property `flatten`
Libraries/FBReactKit/js/react-native-github/Libraries/StyleSheet/StyleSheet.js line 74 col 12-18: property `flatten`
Libraries/FBReactKit/js/react-native-github/Libraries/StyleSheet/StyleSheet.js line 62 col 7-16: statics of StyleSheet

brentvatne · 2015-10-10T01:44:28Z

@vjeux - fixed, although I'm not entirely sure what the type definition should look like here on flatten -- should I just copy and paste it over from flattenStyle? Seems a bit redundant

ide · 2015-10-10T01:45:56Z

Libraries/StyleSheet/StyleSheet.js

space after colon likethe other type decls? also if you wanted to take this further you could maybe write: static flatten: typeof flattenStyle http://flowtype.org/docs/typeof.html

Yup, space after : please

ah that's fantastic thanks @ide :)

vjeux · 2015-10-10T02:01:42Z

Have you tested art from npm, does it work?

brentvatne · 2015-10-10T03:29:15Z

@vjeux - it should work -- before adding art to dependencies we could not require ReactNativeART in react-native.js because it would error out due to missing art module. I'll test installing from this local version tomorrow.

brentvatne · 2015-10-11T01:26:42Z

@vjeux - yup, it works. One thing I wasn't sure about is whether we should have the react-native init project link libART.a automatically -- I left it off for now but could go either way on it. cc @ide

ide · 2015-10-11T04:15:52Z

I could go either way. I lean towards keeping it separate because art is a separate npm package.

brentvatne · 2015-10-11T04:21:26Z

@ide - in this diff I've included art as a dependency for React Native because otherwise we can't add ReactNativeART to the public interface (it can't be required)

ide · 2015-10-11T04:36:46Z

ok. We need the packager to perform better static analysis of imports and cull dead code.

vjeux · 2015-10-12T03:21:54Z

@facebook-github-bot shipit

facebook-github-bot · 2015-10-12T03:22:27Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/182740555392881/int_phab to review.

javache · 2015-10-19T09:29:54Z

@brentvatne Could you rebase this change? Right now it won't merge cleanly.

facebook-github-bot · 2015-10-25T23:12:25Z

@brentvatne updated the pull request.

brentvatne · 2015-10-25T23:12:59Z

@javache - rebased on top of master, should be good to go

javache · 2015-10-25T23:16:10Z

@facebook-github-bot shipit

facebook-github-bot · 2015-10-25T23:17:36Z

Thanks for importing. If you are an FB employee go to https://our.intern.facebook.com/intern/opensource/github/pull_request/182740555392881/int_phab to review.

sebmarkbage · 2015-10-26T17:52:36Z

Libraries/StyleSheet/StyleSheet.js

I would like to move this logic to React and decouple this from styles. The refactoring I've done has decoupled this logic from the style prop.

This is similar to the React.Children.toArray helper so something like that on React might make sense.

It doesn't really hurt to have it here too for now but I don't think it ultimately belongs here.

facebook-github-bot · 2015-11-04T13:13:23Z

@brentvatne updated the pull request.

Summary: - TextInputState as TextInput.State - Touchable - flattenStyle as StyleSheet.flatten - ReactNativeART as ART Original discussion in facebook#1821 Closes facebook#3308 Reviewed By: sebmarkbage Differential Revision: D2527152 Pulled By: javache fb-gh-sync-id: 19d4ef9d4c0e6587b9f0793e1ca624aebb034f3b

facebook-github-bot added GH Review: review-needed CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. labels Oct 9, 2015

brentvatne mentioned this pull request Oct 9, 2015

[Blur & Focus] Support relinquishing focus from multiple views via container #113

Closed

ide assigned vjeux Oct 9, 2015

vjeux reviewed Oct 9, 2015
View reviewed changes

vjeux added GH Review: needs-revision and removed GH Review: review-needed labels Oct 9, 2015

facebook-github-bot added GH Review: review-needed and removed GH Review: needs-revision labels Oct 9, 2015

vjeux added GH Review: needs-revision and removed GH Review: review-needed labels Oct 9, 2015

facebook-github-bot added GH Review: review-needed and removed GH Review: needs-revision labels Oct 10, 2015

ide reviewed Oct 10, 2015
View reviewed changes

javache added the GH Review: accepted label Oct 16, 2015

javache removed the GH Review: review-needed label Oct 16, 2015

javache added GH Review: needs-revision and removed GH Review: accepted labels Oct 20, 2015

brentvatne added 3 commits October 25, 2015 16:11

Expose TextInputState, flattenStyle, Touchable and ReactNativeART

a8e5f06

Fix Flow error on StyleSheet.flatten

8e97613

Update type signature for flatten

6f2656d

brentvatne added GH Review: review-needed and removed GH Review: needs-revision labels Oct 25, 2015

sebmarkbage reviewed Oct 26, 2015
View reviewed changes

brentvatne closed this in 0da2004 Nov 5, 2015

mkonicek mentioned this pull request May 25, 2016

[Blur & Focus] Support relinquishing focus from multiple views via co… #2203

Closed

Conversation

brentvatne commented Oct 9, 2015

Uh oh!

vjeux commented Oct 9, 2015

Uh oh!

facebook-github-bot commented Oct 9, 2015

Uh oh!

vjeux Oct 9, 2015

Choose a reason for hiding this comment

Uh oh!

brentvatne Oct 9, 2015

Choose a reason for hiding this comment

Uh oh!

brentvatne commented Oct 9, 2015

Uh oh!

vjeux commented Oct 9, 2015

Uh oh!

facebook-github-bot commented Oct 9, 2015

Uh oh!

vjeux commented Oct 9, 2015

Uh oh!

brentvatne commented Oct 10, 2015

Uh oh!

ide Oct 10, 2015

Choose a reason for hiding this comment

Uh oh!

vjeux Oct 10, 2015

Choose a reason for hiding this comment

Uh oh!

brentvatne Oct 10, 2015

Choose a reason for hiding this comment

Uh oh!

vjeux commented Oct 10, 2015

Uh oh!

brentvatne commented Oct 10, 2015

Uh oh!

brentvatne commented Oct 11, 2015

Uh oh!

ide commented Oct 11, 2015

Uh oh!

brentvatne commented Oct 11, 2015

Uh oh!

ide commented Oct 11, 2015

Uh oh!

vjeux commented Oct 12, 2015

Uh oh!

facebook-github-bot commented Oct 12, 2015

Uh oh!

javache commented Oct 19, 2015

Uh oh!

facebook-github-bot commented Oct 25, 2015

Uh oh!

brentvatne commented Oct 25, 2015

Uh oh!

javache commented Oct 25, 2015

Uh oh!

facebook-github-bot commented Oct 25, 2015

Uh oh!

sebmarkbage Oct 26, 2015

Choose a reason for hiding this comment

Uh oh!

facebook-github-bot commented Nov 4, 2015

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants