-
Notifications
You must be signed in to change notification settings - Fork 6
Conversation
} | ||
|
||
private void waitForDoubleBackPressed() { | ||
if (doubleBackToExitPressedOnce) { |
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.
What for are you starting new activity instance after single back 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.
i read somewhere that in order to save app state should make a new activity
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.
do you have a better suggestion?
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 should just show toast. If you re-run an activity, the state changing.
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 made some changes, i think it's fine now
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 don't need to start a new activity. Just show the dialog for user. If you start new activity, all user's data is erased.
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.
where am I making a new activity????
are u sure you are checking my last commit?
i do NOT make a new activity in my last comit
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.
In your last commit all fine but can you move this implementation to HashCalculatorFragment
? This needs because HashCalculatorFragment
is a "main" fragment of the app and user can exit by double click just only from it. Thanks.
app/src/main/java/com/smlnskgmail/jaman/hashcheckerlite/MainActivity.java
Outdated
Show resolved
Hide resolved
string resources added
} | ||
|
||
private void waitForDoubleBackPressed() { | ||
if (doubleBackToExitPressedOnce) { |
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 don't need to start a new activity. Just show the dialog for user. If you start new activity, all user's data is erased.
@yasin459 Also, please check Checkstyle issues. You can do this by running next command from a shell: ./gradlew checkstyle |
} | ||
|
||
private void waitForDoubleBackPressed() { | ||
if (doubleBackToExitPressedOnce) { |
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.
In your last commit all fine but can you move this implementation to HashCalculatorFragment
? This needs because HashCalculatorFragment
is a "main" fragment of the app and user can exit by double click just only from it. Thanks.
I made another change, transferred to hashcheckercalculator, and codacy is fixed.anything else? |
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.
Need attention about "Magic Number" in HashCalculatorFragment
.
getString(R.string.action_exit_now), | ||
v -> getActivity().finish() | ||
); | ||
if (doubleBackToExitPressedTwice) { |
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.
Thanks a lot! I'm approve this PR. If you want, you also ca extract time to wait (2000
ms) to the local static constant (its a best practice to extract all variables that can use as constant to a local fields). This is a "Magic Number", you can read more about this here.
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 made magic number, thank you for your advice
Checklist
Common
UI
Logic
Changes
Describe all changes here:
Comments
I checked it and it works good but I would really appreciate any critic or advice