React Native 0.46 #7759
Merged
React Native 0.46 #7759
Conversation
This file contains 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
chrisnojima
commented
Jul 14, 2017
| node: | ||
| version: 5.7.0 | ||
| version: 6 |
There was a problem hiding this comment.
new rn tooling requires some things not in 5
chrisnojima
commented
Jul 14, 2017
| @@ -18,6 +18,7 @@ general: | |||
| build_dir: shared | |||
| dependencies: | |||
| pre: | |||
| - nvm ls-remote | |||
There was a problem hiding this comment.
just so we can see available versions
chrisnojima
commented
Jul 14, 2017
| @@ -7,6 +7,7 @@ | |||
| .*/node_modules/fbjs/.* | |||
| .*/node_modules/is-my-json-valid/.* | |||
| .*/node_modules/json5/.* | |||
| .*/node_modules/metro-bundler/.* | |||
There was a problem hiding this comment.
the rn packager got moved here and has a lot of errors
chrisnojima
commented
Jul 14, 2017
| }) | ||
| .filter(i => !!i) | ||
| // Must do this else we get weird errors from redux-saga, see https://github.com/redux-saga/redux-saga/issues/1000#issuecomment-315180255 | ||
| yield Promise.resolve( |
There was a problem hiding this comment.
have to wrap this in Promise else redux-saga thinks we're returning other generators and gives us yellow boxes
chrisnojima
commented
Jul 14, 2017
| @@ -137,7 +137,7 @@ function* navBasedOnLoginState() { | |||
| } | |||
| } else if (registered) { | |||
| // relogging in | |||
| yield [put.resolve(getExtendedStatus()), put.resolve(getAccounts())] | |||
| yield all([put.resolve(getExtendedStatus()), put.resolve(getAccounts())]) | |||
There was a problem hiding this comment.
redux-saga wants arrays to be wrapped in all()
There was a problem hiding this comment.
There's another spot that causes this warning. I have a fix in a perf commit
chrisnojima
commented
Jul 14, 2017
| @@ -184,6 +185,10 @@ function registerIdentifyUi(): TrackerActionCreator { | |||
| let trackerTimeoutError = 0 | |||
|
|
|||
| const onStart = username => { | |||
| // Don't do this on mobile | |||
There was a problem hiding this comment.
Android complains about long timeouts (over 1m) and likely this doesn't make sense in the mobile context anyways. its not a tracker card that hangs out on the side, it'd be your whole screen. no one is waiting 5 minutes...
chrisnojima
commented
Jul 14, 2017
| @@ -1,7 +1,7 @@ | |||
| // @flow | |||
| import {connector, Main} from './main-shared.native' | |||
| import {compose, lifecycle, withProps} from 'recompose' | |||
| import {NativeBackAndroid} from '../common-adapters/index.native' | |||
| import {NativeBackHandler} from '../common-adapters/index.native' | |||
There was a problem hiding this comment.
This got renamed
chrisnojima
commented
Jul 14, 2017
shared/app/nav.native.js
Outdated
| @@ -109,45 +114,20 @@ const barStyle = ({showStatusBarDarkContent, underStatusBar}) => { | |||
|
|
|||
| function renderStackRoute(route) { | |||
| const {underStatusBar, hideStatusBar, showStatusBarDarkContent} = route.tags | |||
| // Skip extra view if no statusbar | |||
There was a problem hiding this comment.
we actually do need to show it as it's used to tell the native side to hide
chrisnojima
commented
Jul 14, 2017
shared/app/nav.native.js
Outdated
| /> | ||
| {route.component} | ||
| </Box> | ||
| ) | ||
| } | ||
|
|
||
| const forIOS = ({hideNav, shim, tabBar}) => ( |
There was a problem hiding this comment.
We now have a single implementation using the KeyboardAvoidingView (with some different params). We listen for keyboard showing/hiding and hide our tabBar ourselves.
chrisnojima
commented
Jul 14, 2017
| @@ -1,6 +1,6 @@ | |||
| // @flow | |||
| import {RouteDefNode} from '../route-tree' | |||
| import ConvListOrSearch from './conversation-list-or-search' | |||
| import ConvListOrSearch from './conversation-list-or-search.native' | |||
There was a problem hiding this comment.
this is only used by native, so lets mark it as such
chrisnojima
commented
Jul 14, 2017
| @@ -147,7 +147,7 @@ class AvatarRender extends PureComponent<void, Props, State> { | |||
| return ( | |||
| <ClickableBox onClick={onClick} feedback={false}> | |||
| <Box style={boxStyle(size, style)}> | |||
| {skipBackground && | |||
| {!skipBackground && | |||
There was a problem hiding this comment.
this flag was actually the inverse!
chrisnojima
commented
Jul 14, 2017
| @@ -222,7 +222,7 @@ class Input extends Component<void, Props, State> { | |||
| returnKeyType: this.props.returnKeyType, | |||
| value: this.state.value, | |||
| secureTextEntry: this.props.type === 'password', | |||
| underlineColorAndroid: globalColors.transparent, | |||
| underlineColorAndroid: 'transparent', | |||
There was a problem hiding this comment.
suggested by the docs. doesn't seem different but might as well leave it
chrisnojima
commented
Jul 14, 2017
| @@ -88,10 +87,6 @@ function messageCreateComponent(type, key, children, options) { | |||
| } | |||
|
|
|||
| class Markdown extends PureComponent<void, Props, void> { | |||
| shouldComponentUpdate(nextProps: Props): boolean { | |||
There was a problem hiding this comment.
this is likely a leftover, PureComponent does this and new RN complains
chrisnojima
commented
Jul 14, 2017
| @@ -56,7 +56,6 @@ const Render = ({platform, overlay, overlayColor, style}: Props) => { | |||
| <Icon | |||
| type={overlay} | |||
| style={{ | |||
| color: overlayColor, | |||
There was a problem hiding this comment.
this is an image now
chrisnojima
commented
Jul 14, 2017
| @@ -1,7 +1,7 @@ | |||
| // @flow | |||
| import * as shared from './user-bio.shared' | |||
| import React, {Component} from 'react' | |||
| import ReactCSSTransitionGroup from 'react-addons-css-transition-group' | |||
| import {CSSTransitionGroup} from 'react-transition-group' | |||
There was a problem hiding this comment.
this got pulled out of addons and into its own project
chrisnojima
commented
Jul 14, 2017
| @@ -55,15 +54,6 @@ function render() { | |||
| } | |||
|
|
|||
| function load() { | |||
| // Used by material-ui widgets. | |||
There was a problem hiding this comment.
going to entirely kill material-ui after this. doesn't work with react 16
chrisnojima
commented
Jul 14, 2017
|
|
||
| const other = 'Someone else...' | ||
|
|
||
| const UserRow = ({user, onClick}) => ( |
There was a problem hiding this comment.
simple replacement for dropdown. this whole screen should be redone, its very old
chrisnojima
commented
Jul 14, 2017
| @@ -57,7 +52,7 @@ class PrivateEnterUsernameRender extends Component<void, Props, State> { | |||
| ? {notification: {type: 'error', message: customError(this.props.errorText, this.props.errorCode)}} | |||
| : {} | |||
| return ( | |||
| <StandardScreen {...notification}> | |||
| <StandardScreen {...notification} onCancel={this.props.onCancel}> | |||
There was a problem hiding this comment.
standard screen has a headerhoc in it
chrisnojima
commented
Jul 14, 2017
| @@ -160,7 +160,8 @@ dependencies { | |||
| // Manually include GIF support for Fresco b/c it was removed from | |||
| // core library and RN doesn't auto add it back yet (RN 0.30.0) | |||
| // @see DESKTOP-1703 | |||
| compile 'com.facebook.fresco:animated-gif:0.11.0' | |||
| compile 'com.facebook.fresco:animated-base-support:0.14.1' | |||
There was a problem hiding this comment.
need to update this dep else gifs crash rn 0.46
|
The commits 27fa9c1...3b73cee introduce visual changes on linux. 🔎 3 removed, 3 changed
|
|
@keybase/react-hackers |
* remove material-ui/menu * lint * remove material ui entirely! (#7762) * remove material ui! * WIP
|
The commits 27fa9c1...6c2e63e introduce visual changes on linux. 🔎 3 removed, 207 changed
|
|
|
The commits ef8f072...3ef0f53 introduce visual changes on linux. 🔎 3 removed, 207 changed
|
|
@keybase/react-hackers this is ready to look at again. it's been master merged and includes the branches that remove material-ui |
MarcoPolo
approved these changes
Jul 24, 2017
MarcoPolo
approved these changes
Jul 24, 2017
|
The commits 617b2c3...2401d42 introduce visual changes on linux. 🔎 3 removed, 207 changed
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR updates our RN to 0.46. This also means our react dependency got updated to 16 alpha which affects desktop also.
Additionally the update to 0.46 updates some tooling which allows us to see the dimensions of views in the react devtools which led me to discover some high level layout problems. This PR also addresses this. Before we had flex:1 screens which (depending on the contents) push the screens below the fold. This lead to bugs about things being cut off.
This is split into several commits:
notes:
Plan for Addons in React 16 facebook/react#9207
No react-perf on desktop :(