Skip to content

Brand crash dialog, add crash trigger if debug - #4982

Merged
timrae merged 1 commit into
ankidroid:masterfrom
mikehardy:crash-dialog-branding
Sep 20, 2018
Merged

Brand crash dialog, add crash trigger if debug#4982
timrae merged 1 commit into
ankidroid:masterfrom
mikehardy:crash-dialog-branding

Conversation

@mikehardy

@mikehardy mikehardy commented Sep 18, 2018

Copy link
Copy Markdown
Member

Purpose / Description

Previously I've been relying on a personal collection of
crash bugs to test the crash dialog, but that's a poor testing
experience - now we have an advanced preference area to just crash

During the ACRA upgrade #4893 some branding was already broken, and
the rest was lost, when I finally saw the dialog again with
fresh eyes on my son's tablet it was terrible so this puts the
icon and title where they belong

Fixes

No issue logged, just fixed it

Approach

This adds a beautiful icon and nice text

How Has This Been Tested?

I am getting better at UI things so I added an advanced preference to trigger crashes, in debug mode only. I tested the pref was disabled in release builds and then tested the crash - debug/release + pre/post images attached

It does add one Codacy warning because I throw RuntimeException in the trigger

Checklist

Please, go through these checks before submitting the PR.

  • You have not changed whitespace unnecessarily (it makes diffs hard to read)
  • You have a descriptive commit message with a short title (first line, max 50 chars).
  • Your code follows the style of the project (e.g. never omit braces in if statements)
  • You have commented your code, particularly in hard-to-understand areas
  • You have performed a self-review of your own code

ankidroid_advanced_pref_release_no_trigger
ankidroid_advanced_pref_debug_crash_trigger
ankidroid_crash_dialog_no_branding
ankidroid_crash_dialog_branded

@mikehardy mikehardy added this to the v2.9 release milestone Sep 18, 2018
@mikehardy
mikehardy requested a review from timrae September 18, 2018 23:48
@mikehardy

Copy link
Copy Markdown
Member Author

Travis CI is having some sort of non-code-related crashing going on. May have to restart all these PR builds some other time, they're working on my Travis and locally

@mikehardy

Copy link
Copy Markdown
Member Author

Travis is fixed - I had to delete the caches on the PRs with weird errors and that resolved it. Unfortunately each of my last 3 PRs have Codacy problems (UI tests are long, a crash trigger is a raw exception, and API level guarding is a weird nested-if) but I did work hard to maintain quality and I look forward to seeing what you think. Sorry I'm bombing you but I'm fine with review as you have time, of course :-)

@timrae timrae left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you add the preference programmatically instead of in XML to avoid confusion? And I don't want these strings to be translated so just hard code them in.

@mikehardy

Copy link
Copy Markdown
Member Author

Definitely, good idea

@timrae

timrae commented Sep 20, 2018

Copy link
Copy Markdown
Member

Icon looks great btw :)

Previously I've been relying on a personal collection of
crash bugs to test the crash dialog, but that's a poor testing
experience - now we have an advanced preference area to just crash

During the ACRA upgrade some branding was already broken, and
the rest was lost, when I finally saw the dialog again with
fresh eyes it was terrible so this puts the icon and title back
@mikehardy
mikehardy force-pushed the crash-dialog-branding branch from 5238b43 to 2bbd79e Compare September 20, 2018 06:59
@mikehardy

Copy link
Copy Markdown
Member Author

re-pushing now - it was good practice for my nascent android UI skills, but the PR is even smaller now. Re-tested on debug and release builds, pref is shown or not shown as expected.

@ankidroid ankidroid deleted a comment Sep 20, 2018
@ankidroid ankidroid deleted a comment Sep 20, 2018
@ankidroid ankidroid deleted a comment from mikehardy Sep 20, 2018
@timrae
timrae merged commit da1f19c into ankidroid:master Sep 20, 2018
@timrae

timrae commented Sep 20, 2018

Copy link
Copy Markdown
Member

I would have preferred the new preference and the icon to be in separate commits, but it's not a big deal... Looking good!

@mikehardy
mikehardy deleted the crash-dialog-branding branch September 20, 2018 07:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants