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

Add option to re-prompt at intervals after user accept/ignore #21

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add option to re-prompt at intervals after user accept/ignore #21

wants to merge 2 commits into from

Conversation

sampengilly
Copy link

Allow the prompt to be shown again at intervals after each user action (accept/ignore).

Track the time each time the AgreeShowDialog preference is set to false. When checking if this value is false, or it with the result of isOverLastAgreeShowFalseDate() to determine if it has been X time units since AgreeShowDialog was set to false, a result of true will continue with the rest of the evaluation.

Defaults to off. Setting a time unit will turn the check on. Date of AgreeShowDialog = false is always tracked

return getAgreeShowDialog() &&
// If Agree show is false (user has ignored/accepted) then return false unless it has
// been `repromptDate` time since the time when the user accepted/ignored the prompt
return (getAgreeShowDialog() || isOverLastAgreeShowFalseDate()) &&
Copy link

Choose a reason for hiding this comment

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

'&&' should be on a new line.

Copy link
Author

Choose a reason for hiding this comment

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

Matching existing style as much as possible while combining the tightly related ||

@@ -40,6 +40,8 @@

private static final String PREF_KEY_IS_AGREE_SHOW_DIALOG = "androidrate_is_agree_show_dialog";

private static final String PREF_KEY_LAST_AGREE_SHOW_FALSE_DATE = "androidrate_last_agree_show_false_date";
Copy link

Choose a reason for hiding this comment

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

Line is longer than 100 characters (found 111).

Copy link
Author

Choose a reason for hiding this comment

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

Not the only one among these constants

@@ -36,6 +35,7 @@
import static com.vorlonsoft.android.rate.PreferenceHelper.getCustomEventCount;
import static com.vorlonsoft.android.rate.PreferenceHelper.getInstallDate;
import static com.vorlonsoft.android.rate.PreferenceHelper.getIsAgreeShowDialog;
import static com.vorlonsoft.android.rate.PreferenceHelper.getLastAgreeShowFalseDate;
Copy link

Choose a reason for hiding this comment

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

Wrong lexicographical order for 'com.vorlonsoft.android.rate.PreferenceHelper.getLastAgreeShowFalseDate' import. Should be before 'java.util.Map'.

Copy link
Author

Choose a reason for hiding this comment

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

Matching existing imports in alphabetical order

@codeclimate
Copy link

codeclimate bot commented Jan 8, 2020

Code Climate has analyzed commit 57d7b50 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Style 3

View more on Code Climate.

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.

None yet

1 participant