Skip to content
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

Updated RedBox screen #22242

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions RNTester/RNTester/Info.plist
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@
<!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd">
<plist version="1.0">
<dict>
<key>UIStatusBarStyle</key>
<string>UIStatusBarStyleBlackTranslucent</string>
<key>CFBundleDevelopmentRegion</key>
<string>en</string>
<key>CFBundleExecutable</key>
Expand Down
78 changes: 55 additions & 23 deletions React/Modules/RCTRedBox.m
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ - (instancetype)initWithFrame:(CGRect)frame
#else
self.windowLevel = UIWindowLevelStatusBar - 1;
#endif
self.backgroundColor = [UIColor colorWithRed:0.8 green:0 blue:0 alpha:1];
self.backgroundColor = [UIColor colorWithRed:0.1 green:0.1 blue:0.1 alpha:1];
self.hidden = YES;

UIViewController *rootController = [UIViewController new];
Expand All @@ -58,7 +58,7 @@ - (instancetype)initWithFrame:(CGRect)frame
const CGFloat buttonHeight = 60;

CGRect detailsFrame = rootView.bounds;
detailsFrame.size.height -= buttonHeight;
detailsFrame.size.height -= buttonHeight + [self bottomSafeViewHeight];

_stackTraceTableView = [[UITableView alloc] initWithFrame:detailsFrame style:UITableViewStylePlain];
_stackTraceTableView.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleHeight;
Expand All @@ -73,10 +73,10 @@ - (instancetype)initWithFrame:(CGRect)frame
[rootView addSubview:_stackTraceTableView];

#if TARGET_OS_SIMULATOR
NSString *reloadText = @"Reload JS (\u2318R)";
NSString *dismissText = @"Dismiss (ESC)";
NSString *copyText = @"Copy (\u2325\u2318C)";
NSString *extraText = @"Extra Info (\u2318E)";
NSString *reloadText = @"Reload\n(\u2318R)";
NSString *dismissText = @"Dismiss\n(ESC)";
NSString *copyText = @"Copy\n(\u2325\u2318C)";
NSString *extraText = @"Extra Info\n(\u2318E)";
#else
NSString *reloadText = @"Reload JS";
NSString *dismissText = @"Dismiss";
Expand All @@ -88,53 +88,85 @@ - (instancetype)initWithFrame:(CGRect)frame
dismissButton.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleTopMargin | UIViewAutoresizingFlexibleRightMargin;
dismissButton.accessibilityIdentifier = @"redbox-dismiss";
dismissButton.titleLabel.font = [UIFont systemFontOfSize:13];
dismissButton.titleLabel.lineBreakMode = NSLineBreakByWordWrapping;
dismissButton.titleLabel.textAlignment = NSTextAlignmentCenter;
dismissButton.backgroundColor = [UIColor colorWithRed:0.1 green:0.1 blue:0.1 alpha:1];
[dismissButton setTitle:dismissText forState:UIControlStateNormal];
[dismissButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateNormal];
[dismissButton setTitleColor:[UIColor whiteColor] forState:UIControlStateHighlighted];
[dismissButton setTitleColor:[UIColor whiteColor] forState:UIControlStateNormal];
[dismissButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateHighlighted];
[dismissButton addTarget:self action:@selector(dismiss) forControlEvents:UIControlEventTouchUpInside];

UIButton *reloadButton = [UIButton buttonWithType:UIButtonTypeCustom];
reloadButton.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleTopMargin | UIViewAutoresizingFlexibleLeftMargin;
reloadButton.accessibilityIdentifier = @"redbox-reload";
reloadButton.titleLabel.font = [UIFont systemFontOfSize:13];

reloadButton.titleLabel.lineBreakMode = NSLineBreakByWordWrapping;
reloadButton.titleLabel.textAlignment = NSTextAlignmentCenter;
reloadButton.backgroundColor = [UIColor colorWithRed:0.1 green:0.1 blue:0.1 alpha:1];
[reloadButton setTitle:reloadText forState:UIControlStateNormal];
[reloadButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateNormal];
[reloadButton setTitleColor:[UIColor whiteColor] forState:UIControlStateHighlighted];
[reloadButton setTitleColor:[UIColor whiteColor] forState:UIControlStateNormal];
[reloadButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateHighlighted];
[reloadButton addTarget:self action:@selector(reload) forControlEvents:UIControlEventTouchUpInside];

UIButton *copyButton = [UIButton buttonWithType:UIButtonTypeCustom];
copyButton.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleTopMargin | UIViewAutoresizingFlexibleLeftMargin;
copyButton.accessibilityIdentifier = @"redbox-copy";
copyButton.titleLabel.font = [UIFont systemFontOfSize:13];
copyButton.titleLabel.lineBreakMode = NSLineBreakByWordWrapping;
copyButton.titleLabel.textAlignment = NSTextAlignmentCenter;
copyButton.backgroundColor = [UIColor colorWithRed:0.1 green:0.1 blue:0.1 alpha:1];
[copyButton setTitle:copyText forState:UIControlStateNormal];
[copyButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateNormal];
[copyButton setTitleColor:[UIColor whiteColor] forState:UIControlStateHighlighted];
[copyButton setTitleColor:[UIColor whiteColor] forState:UIControlStateNormal];
[copyButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateHighlighted];
[copyButton addTarget:self action:@selector(copyStack) forControlEvents:UIControlEventTouchUpInside];
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@kelset Also ideally all those buttons should be in a func that just takes a param and returns a UIButton.

This would clean up the code a lot, but I haven’t done Obj-C in a while. Let me know if that makes sense, and if something like would work.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's okay for now. Refactoring the buttons is probably outside of the scope of this change


UIButton *extraButton = [UIButton buttonWithType:UIButtonTypeCustom];
extraButton.autoresizingMask = UIViewAutoresizingFlexibleWidth | UIViewAutoresizingFlexibleTopMargin | UIViewAutoresizingFlexibleLeftMargin;
extraButton.accessibilityIdentifier = @"redbox-extra";
extraButton.titleLabel.font = [UIFont systemFontOfSize:13];
extraButton.titleLabel.lineBreakMode = NSLineBreakByWordWrapping;
extraButton.titleLabel.textAlignment = NSTextAlignmentCenter;
extraButton.backgroundColor = [UIColor colorWithRed:0.1 green:0.1 blue:0.1 alpha:1];
[extraButton setTitle:extraText forState:UIControlStateNormal];
[extraButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateNormal];
[extraButton setTitleColor:[UIColor whiteColor] forState:UIControlStateHighlighted];
[extraButton setTitleColor:[UIColor whiteColor] forState:UIControlStateNormal];
[extraButton setTitleColor:[UIColor colorWithWhite:1 alpha:0.5] forState:UIControlStateHighlighted];
[extraButton addTarget:self action:@selector(showExtraDataViewController) forControlEvents:UIControlEventTouchUpInside];

CGFloat buttonWidth = self.bounds.size.width / 4;
dismissButton.frame = CGRectMake(0, self.bounds.size.height - buttonHeight, buttonWidth, buttonHeight);
reloadButton.frame = CGRectMake(buttonWidth, self.bounds.size.height - buttonHeight, buttonWidth, buttonHeight);
copyButton.frame = CGRectMake(buttonWidth * 2, self.bounds.size.height - buttonHeight, buttonWidth, buttonHeight);
extraButton.frame = CGRectMake(buttonWidth * 3, self.bounds.size.height - buttonHeight, buttonWidth, buttonHeight);
CGFloat bottomButtonHeight = self.bounds.size.height - buttonHeight - [self bottomSafeViewHeight];

dismissButton.frame = CGRectMake(0, bottomButtonHeight, buttonWidth, buttonHeight);
reloadButton.frame = CGRectMake(buttonWidth, bottomButtonHeight, buttonWidth, buttonHeight);
copyButton.frame = CGRectMake(buttonWidth * 2, bottomButtonHeight, buttonWidth, buttonHeight);
extraButton.frame = CGRectMake(buttonWidth * 3, bottomButtonHeight, buttonWidth, buttonHeight);

UIView *topBorder = [[UIView alloc] initWithFrame:CGRectMake(0, bottomButtonHeight + 1, rootView.frame.size.width, 1)];
topBorder.backgroundColor = [UIColor colorWithRed:0.70 green:0.70 blue:0.70 alpha:1.0];

[rootView addSubview:dismissButton];
[rootView addSubview:reloadButton];
[rootView addSubview:copyButton];
[rootView addSubview:extraButton];
[rootView addSubview:topBorder];

UIView *bottomSafeView = [UIView new];
bottomSafeView.backgroundColor = [UIColor colorWithRed:0.1 green:0.1 blue:0.1 alpha:1];
bottomSafeView.frame = CGRectMake(0, self.bounds.size.height - [self bottomSafeViewHeight], self.bounds.size.width, [self bottomSafeViewHeight]);

[rootView addSubview:bottomSafeView];
}
return self;
}

- (NSInteger)bottomSafeViewHeight
{
if (@available(iOS 11.0, *)) {
return [UIApplication sharedApplication].delegate.window.safeAreaInsets.bottom;
} else {
return 0;
}
}

Copy link
Contributor Author

@Monte9 Monte9 Nov 17, 2018

Choose a reason for hiding this comment

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

@kelset this is not ideal as the code is redundant with RCTDeviceInfo.

RCTDeviceInfo has a func called RCTIsIPhoneX but since I am not very proficient in Obj-C, I couldn't figure out how to reuse it. Any suggestions on how to reuse/improve this code would be appreciated!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah there's no easy way to reuse that function unless we extract it out to a util module.

I think this could work: safeAreaInsets: according to the docs, if the rootView is part of the view hierarchy, this value should return the extra padding we need to add to our UI.

@shergin what's the best way to do this?

Copy link
Contributor

Choose a reason for hiding this comment

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

@Monte9 do you mind following up on @frantic's last comment? I think that's the only remaining blocker to getting this shipped :)

Copy link
Contributor Author

@Monte9 Monte9 Dec 10, 2018

Choose a reason for hiding this comment

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

@cpojer Yes, will update it tonight. Was waiting for @shergin to respond. 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@frantic quick follow up: when I try to use the safeAreaInsets on the rootview I get 0 on an iPhoneX.

NSLog(@"safeArea: %f", rootView.safeAreaInsets.bottom);
> 2018-12-13 14:42:24.955719-0600 RNTester[37397:1022268] safeArea: 0.000000

I looked up a few issues on stack overflow and I figure out that I need to do this instead:

[UIApplication sharedApplication].delegate.window.safeAreaInsets.bottom;

Hence I came up with this which is pretty abstracted and much better than what I had before. Thanks for the suggestion. 👍

- (NSInteger)bottomSafeViewHeight
{
    if (@available(iOS 11.0, *)) {
        return [UIApplication sharedApplication].delegate.window.safeAreaInsets.bottom;
    } else {
        return 0;
    }
}

RCT_NOT_IMPLEMENTED(- (instancetype)initWithCoder:(NSCoder *)aDecoder)

- (void)dealloc
Expand Down Expand Up @@ -247,14 +279,14 @@ - (UITableViewCell *)tableView:(UITableView *)tableView cellForRowAtIndexPath:(N
- (UITableViewCell *)reuseCell:(UITableViewCell *)cell forErrorMessage:(NSString *)message
{
if (!cell) {
cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:@"msg-cell"];
cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleDefault reuseIdentifier:@"msg-cell"];
cell.textLabel.accessibilityIdentifier = @"redbox-error";
cell.textLabel.textColor = [UIColor whiteColor];
cell.textLabel.font = [UIFont boldSystemFontOfSize:16];
cell.textLabel.lineBreakMode = NSLineBreakByWordWrapping;
cell.textLabel.numberOfLines = 0;
cell.detailTextLabel.textColor = [UIColor whiteColor];
cell.backgroundColor = [UIColor clearColor];
cell.backgroundColor = [UIColor colorWithRed:0.82 green:0.10 blue:0.15 alpha:1.0];
cell.selectionStyle = UITableViewCellSelectionStyleNone;
}

Expand All @@ -267,11 +299,11 @@ - (UITableViewCell *)reuseCell:(UITableViewCell *)cell forStackFrame:(RCTJSStack
{
if (!cell) {
cell = [[UITableViewCell alloc] initWithStyle:UITableViewCellStyleSubtitle reuseIdentifier:@"cell"];
cell.textLabel.textColor = [UIColor colorWithWhite:1 alpha:0.9];
cell.textLabel.textColor = [UIColor whiteColor];
cell.textLabel.font = [UIFont fontWithName:@"Menlo-Regular" size:14];
cell.textLabel.lineBreakMode = NSLineBreakByCharWrapping;
cell.textLabel.numberOfLines = 2;
cell.detailTextLabel.textColor = [UIColor colorWithWhite:1 alpha:0.7];
cell.detailTextLabel.textColor = [UIColor colorWithRed:0.70 green:0.70 blue:0.70 alpha:1.0];
cell.detailTextLabel.font = [UIFont fontWithName:@"Menlo-Regular" size:11];
cell.detailTextLabel.lineBreakMode = NSLineBreakByTruncatingMiddle;
cell.backgroundColor = [UIColor clearColor];
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
<?xml version="1.0" encoding="utf-8"?>
<layer-list xmlns:android="http://schemas.android.com/apk/res/android" >
<item>
<shape android:shape="rectangle" >
<solid android:color="#1A1A1A" />
</shape>
</item>

<item android:bottom="-2dp" android:right="-2dp" android:left="-2dp">
<shape>
<solid android:color="@android:color/transparent" />
<stroke
android:width="1dp"
android:color="#B3B3B3" />
</shape>
</item>
</layer-list>
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
android:id="@+id/rn_frame_file"
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:textColor="#E6B8B8"
android:textColor="#B3B3B3"
android:textSize="12sp"
android:fontFamily="monospace"
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@
android:textColor="@android:color/white"
android:textSize="16sp"
android:textStyle="bold"
android:background="#D01926"
/>
3 changes: 2 additions & 1 deletion ReactAndroid/src/main/res/devsupport/layout/redbox_view.xml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical"
android:background="#E80000"
android:background="#1A1A1A"
>
<ListView
android:id="@+id/rn_redbox_stack"
Expand Down Expand Up @@ -53,6 +53,7 @@
android:layout_width="match_parent"
android:layout_height="wrap_content"
android:orientation="horizontal"
android:background="@drawable/redbox_top_border_background"
>
<Button
android:id="@+id/rn_redbox_dismiss_button"
Expand Down
2 changes: 1 addition & 1 deletion ReactAndroid/src/main/res/devsupport/values/strings.xml
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
<string name="catalyst_dismiss_button" project="catalyst" translatable="false">Dismiss\n(ESC)</string>
<string name="catalyst_reload_button" project="catalyst" translatable="false">Reload\n(R,\u00A0R)</string>
<string name="catalyst_poke_sampling_profiler" project="catalyst" translatable="false">Start/Stop Sampling Profiler</string>
<string name="catalyst_copy_button" project="catalyst" translatable="false">Copy</string>
<string name="catalyst_copy_button" project="catalyst" translatable="false">Copy\n</string>
<string name="catalyst_report_button" project="catalyst" translatable="false">Report</string>
<string name="catalyst_loading_from_url" project="catalyst" translatable="false">Loading from %1$s…</string>
</resources>