Skip to content

Task 001 solution: fix Placeholder RuntimeException#2

Open
HelloImKevo wants to merge 1 commit into
masterfrom
pr-demo
Open

Task 001 solution: fix Placeholder RuntimeException#2
HelloImKevo wants to merge 1 commit into
masterfrom
pr-demo

Conversation

@HelloImKevo
Copy link
Copy Markdown
Owner

@HelloImKevo HelloImKevo commented Mar 26, 2020

Summary:

This is my proposed solution for BRD Task #1. I changed the switch case logic to remove the logging
statements and RuntimeException (when a Placeholder button is clicked) and instead show a
"Under Construction" dialog. How does it look?

Test Plan:

  • Deploy the Groovy app to a Pixel 2 emulator.
  • Click on the Log In button.
  • On the Main Dashboard screen, click on either of the "Placeholder" buttons.
  • Confirm the app doesn't crash.

Additional Thoughts:

I removed some of the logs that are published to logcat, because they're basically "TODO" placeholder
verbiage. However, maybe these would be beneficial to keep around.
Also, since the RuntimeException has been removed, it now just shows a Placeholder Alert Dialog
with text that isn't very helpful to the user. Maybe we should remove the buttons from the UI instead?

Related Issue(s):

https://jira.imobile3.com/browse/ABC-1234

Comment on lines -92 to -98
LogHelper.writeWithTrace(Level.INFO, TAG, "Management area not implemented");
break;

case TimeTracking:
LogHelper.writeWithTrace(Level.INFO, TAG, "Time Tracking area not implemented");
break;

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

I was considering leaving the log messages in place for the non-Placeholder buttons, what do you guys think?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why do you think they need to be removed? I think they can stay.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Hm... After further consideration, I concur! 👍

import android.os.Bundle;

import com.imobile3.groovypayments.R;
import com.imobile3.groovypayments.logging.LogHelper;
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Maybe it would be better to retain the LogHelper import. Thoughts?

HelloImKevo pushed a commit that referenced this pull request Apr 8, 2020
* [Android] Initial UI implementation for Daily Reports

* Added dimen value to the icon width and height

* Code formate

* revision changes
HelloImKevo pushed a commit that referenced this pull request Apr 8, 2020
* [Android] Initial UI implementation for Daily Reports

* Added dimen value to the icon width and height

* Code formate

* revision changes
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