Skip to content
This repository has been archived by the owner on Oct 17, 2021. It is now read-only.

Feature #65

Merged
merged 6 commits into from May 17, 2021
Merged

Feature #65

merged 6 commits into from May 17, 2021

Conversation

vipozdn
Copy link
Contributor

@vipozdn vipozdn commented May 12, 2021

Checklist

Common

  • I am ran the app before creating PR
  • I am ran all tests before creating PR

UI

  • I am ran the app for visual checks.

Logic

  • I am tested basic app functionality.

Changes

Describe all changes here:

  • First;
  • Second;
  • Third;
  • ...

Comments

Describe all additional information here.

1. Added:
- method that opens Google Play link in WebUtils class
1. Added:
- canShowRateAppDialog, increaseHashGenerationCount, save and
getIntPreference methods to SharedPreferencesSettingsHelper class;
- key_hash_generation_count string to strings-non-translatable.
1. Added:
- AppAlertDialog that shows Rate App dialog in HashCalculatorFragment;
- rate app strings to strings.xml.
@fartem fartem self-requested a review May 13, 2021 07:17
@fartem fartem linked an issue May 13, 2021 that may be closed by this pull request
Copy link
Member

@fartem fartem left a comment

Choose a reason for hiding this comment

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

Update SettingsHelper injection in HashCalculatorFragment.

@@ -97,6 +100,24 @@
);
} else {
etGeneratedHash.setText(hashValue);
SharedPreferencesSettingsHelper settingsHelper = new SharedPreferencesSettingsHelper(context);
Copy link
Member

Choose a reason for hiding this comment

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

You need to use SettingsHelper instance from Dagger injection. If a class don't contains SettingsHelper injection, add it as local public field with @Inject annotation.

@fartem fartem added the enhancement New feature or request label May 13, 2021
@fartem
Copy link
Member

fartem commented May 13, 2021

@vipozdn Also, you can copy instrumentation tests for Rate dialog from here.

1. Added:
- usage of SettingsHelper instance from Dagger injection in HashCalculatorFragment;
- declaration of methods from SharedPreferencesSettingsHelper class to SettingsHelper interface;
- RateAppDialogTest class extending BaseUITest.
@@ -41,4 +43,7 @@

void setRefreshSelectedFileStatus(boolean status);

boolean canShowRateAppDialog(Context context);
Copy link
Member

Choose a reason for hiding this comment

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

Delete Context argument from canShowRateAppDialog and increaseHashGenerationCount methods. Context available in interface realization (SharedPreferencesSettingsHelper).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay!

@fartem
Copy link
Member

fartem commented May 14, 2021

@vipozdn Also, check Codacy report. In last commit you have some code issues.

3. Updated:
- documentation by fartem.

4. Deleted:
- unused import at HashCalculatorFragment class.
1. Deleted:
- Context argument from canShowRateAppDialog and increaseHashGenerationCount methods at SettingsHelper, SharedPreferencesSettingsHelper and HashCalculatorFragment;
- unused import at SettingsHelper class.
@fartem fartem merged commit ac49311 into hash-checker:master May 17, 2021
@fartem
Copy link
Member

fartem commented May 17, 2021

@vipozdn thanks.

@vipozdn
Copy link
Contributor Author

vipozdn commented May 17, 2021

@vipozdn thanks.

I'm always happy to help!

@fartem
Copy link
Member

fartem commented May 18, 2021

@vipozdn Also, your PR contains some issues from CPD. You can fix issues and assign this issue. CPD checks run with cpdCheck task by Gradle.

@vipozdn
Copy link
Contributor Author

vipozdn commented May 18, 2021

@vipozdn Also, your PR contains some issues from CPD. You can fix issues and assign this issue. CPD checks run with cpdCheck task by Gradle.

Okay

@vipozdn vipozdn deleted the feature branch May 19, 2021 13:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rate app dialog
2 participants