Closes #1039: Adds crash reporting #1053
Conversation
ghost
assigned 
sblatz
force-pushed
the
1039
branch
4 times, most recently
from
a4ebe21
to
2d76a28
Compare
| @@ -0,0 +1,383 @@ | |||
| <vector xmlns:android="http://schemas.android.com/apk/res/android" | |||
| xmlns:aapt="http://schemas.android.com/aapt" | |||
| android:width="300dp" | |||
There was a problem hiding this comment.
300dp x 300dp is getting a bit iffy for a VectorDrawable because of the rasterization performance penalty. We may want to consider PNGs. https://developer.android.com/studio/write/vector-asset-studio.html#when
| android:layout_height="match_parent" | ||
| android:background="@color/photonGrey10"> | ||
|
|
||
| <View |
There was a problem hiding this comment.
If this image isn't important for accessibility, we should mark it not important for accessibility. Also, it feels odd to use a View with a background instead of an ImageView with a src attribute. One gets additional fields with an ImageView, like scaleType, adjustViewBounds, and other transformations. Background always scales to the size of the View.
| android:textSize="14sp" | ||
| app:layout_constraintBottom_toBottomOf="parent" | ||
| app:layout_constraintEnd_toEndOf="parent" | ||
| app:layout_constraintWidth_percent=".4" /> |
There was a problem hiding this comment.
We should be consistent with style. With leading zero or without.
|
Fixed nits :) |
sblatz
force-pushed
the
1039
branch
3 times, most recently
from
8a582a3
to
91649a5
Compare
| @@ -0,0 +1,96 @@ | |||
| <?xml version="1.0" encoding="utf-8"?> | |||
| android:id="@+id/crash_tab_image" | ||
| android:layout_width="0dp" | ||
| android:layout_height="0dp" | ||
| android:layout_marginTop="8dp" |
There was a problem hiding this comment.
Could we pull any of these values into dimens?
| android:singleLine="false" | ||
| android:text="@string/tab_crash_title" | ||
| android:textAlignment="center" | ||
| android:textColor="@android:color/black" |
There was a problem hiding this comment.
Do we need to define this textColor as black? This should be pulled from styles?
In the interest of keeping the diffs for this feature smaller there are a few TODO's and unimplemented logic in this PR (handling of session restoration and Telemetry)
Note that these should all be squashed when merged, as the last commit is purely for my convenience in continuing to build out this feature