-
Notifications
You must be signed in to change notification settings - Fork 1.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updates react native to 0.34 and cleans up the dev menu #4453
Conversation
chrisnojima
commented
Sep 28, 2016
- qr code scanning is busted on ios 10. This happens on master. They've fixed some stuff but its still flakey. See Crash on second invocation react-native-camera/react-native-camera#426
- android works
- removed some unneeded android overlay intent stuff
- iOS works
- iOS changed all the swift code to objc so its closer to its RN counterpart. Also used some new RN helpers to cleanup the bundle location code. Removed the bridging test code and also the native devmenu stuff (we haven't used that in awhile)
…to plist. qr code still crashes if we render it twice due to a known bug
@@ -33,11 +33,6 @@ | |||
@TargetApi(Build.VERSION_CODES.KITKAT) | |||
protected void onCreate(Bundle savedInstanceState) { | |||
logFile = this.getFileStreamPath("android.log"); | |||
if (android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.M && !Settings.canDrawOverlays(this) && this.getUseDeveloperSupport()) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
seemingly works fine without this. This is setup in other RN code so i think we don't need to do this ourselves anymore
@@ -1,42 +0,0 @@ | |||
import Foundation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
@@ -1,30 +0,0 @@ | |||
import Foundation |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
|
||
NSURL *jsCodeLocation; | ||
|
||
jsCodeLocation = [[RCTBundleURLProvider sharedSettings] jsBundleURLForBundleRoot:@"index.ios" fallbackResource:nil]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
way simpler
@@ -58,5 +58,7 @@ | |||
<string>63ff6926-88f0-4db0-8886-a4bb56bd4ad5</string> | |||
<key>UIViewControllerBasedStatusBarAppearance</key> | |||
<false/> | |||
<key>NSCameraUsageDescription</key> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
needed on ios 10
// Copyright © 2016 Keybase. All rights reserved. | ||
// | ||
|
||
#import "KeyListener.h" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is split out from the app delegate code
@@ -1,67 +0,0 @@ | |||
// |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👊
@@ -11,6 +11,7 @@ class QR extends Component<void, Props, void> { | |||
return ( | |||
<Camera | |||
style={{...cameraStyle, ...this.props.style}} | |||
captureAudio={false} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this should have always been set. in ios 10 you get extra microphone popups w/o this
@keybase/react-hackers |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was npm-shrinkwrap
removed intentionally? I think this needs a re-shrinkwrap.
} | ||
|
||
- (void)goBackInTime:(UIKeyCommand *)sender { | ||
[self.bridge.eventDispatcher sendAppEventWithName:@"backInTime" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see references to these events in shared/index.native.js
. Can they be removed now?
Looks good other than the two comments above! |
Shrink wrap was a mistake. I'll fix that tomorrow. On Wednesday, September 28, 2016, Max Goodman notifications@github.com
|
…ne in dumb filter. add setting the num of components to show
ok so i cleaned up the dev menu up a bunch. did more stuff in the store. removed items we don't really use anymore. also removed the stylesheet and ported that over to the dumbsheet so we just have one thing now. |
@@ -24,11 +24,7 @@ module.exports = { | |||
loader: 'json', | |||
}, { | |||
test: /\.(gif|png)$/, | |||
loader: 'file', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is pretty slow so i removed it. less safe but takes half the time to webpack build
@@ -179,16 +176,4 @@ function getCurrentStatus (): AsyncAction { | |||
} | |||
} | |||
|
|||
export function getDevSettings () { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🔥
import {TabBarButton, TabBarItem} from './tab-bar' | ||
import {globalStyles, globalColors} from '../styles' | ||
import {iconMeta} from './icon.constants' | ||
|
||
const onCheck = () => console.log('on check!') | ||
const onClick = () => console.log('on click!') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is me finally moving the contents of the stylesheet over to the dumbsheet
|
||
class DevMenu extends Component { | ||
render () { | ||
const menuItems = [ | ||
{name: 'Login', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
removing items we don't use anymore
@@ -1,46 +0,0 @@ | |||
// @flow | |||
import CommonMap from '../common-adapters/dumb.desktop' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is just moved into the dumb-sheet subfolder
@@ -33,8 +33,9 @@ class Render extends Component<void, Props, any> { | |||
} | |||
|
|||
render () { | |||
const filter = this.props.dumbFilter.toLowerCase() | |||
let numItemsLeftWeCanShow = 10 | |||
const parts = this.props.dumbFilter.toLowerCase().split(':') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can enter something like buttons:20
to see 20 buttons render now
@@ -55,7 +56,7 @@ class Render extends Component<void, Props, any> { | |||
.map((mockKey, idx) => { | |||
--numItemsLeftWeCanShow | |||
|
|||
if (numItemsLeftWeCanShow <= 0) return null | |||
if (numItemsLeftWeCanShow < 0) return null |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
off by 1 error!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! We should split up the common-adapters dumb.js in the future. It's now over 500 lines long.
Once this lands we can consider revisiting #4245 |
but we're going to switch fonts soon right |
|
🔎 The commits e4fea7c30ce7...4b8ba46b2555 introduced some visual changes on linux:
|