Add funding reminder #8384
Add funding reminder #8384
Conversation
(cherry picked from commit 35dd88b2423a5de98525dc40dc95dff149868baa)
wmontwe
force-pushed
the
add-funding-reminder
branch
2 times, most recently
from
8635230
to
67d7058
Compare
(cherry picked from commit 1c278dbbaa005f22bbac6672d7699ee34913e518)
wmontwe
force-pushed
the
add-funding-reminder
branch
from
67d7058
to
7dfef3a
Compare
| import com.fsck.k9.feature.featureLauncherModule | ||
| import org.koin.dsl.module | ||
|
|
||
| val featureModule = module { |
There was a problem hiding this comment.
Should this use the plural form (featuresModule) considering this combines multiple features?
| return Intent(intent).apply { | ||
| data = FundingRoute.Contribution.toDeepLinkUri() | ||
| } |
There was a problem hiding this comment.
This couples the feature to the existing FeatureLauncherActivity implementation detail of using a separate activity with a base intent and a navigation path in the data field. Instead, we could use a callback and leave it up to the module using this feature module on how to launch the action when the user presses the "Yes" button.
The callback could then use FeatureLauncherActivity.getIntent(context, target = FeatureLauncherTarget.Funding) like we already do in SettingsListFragment.
| override fun getFundingType(): FundingType { | ||
| return FundingType.GOOGLE_PLAY | ||
| } | ||
|
|
||
| override fun addFundingReminder(activity: AppCompatActivity, launcherIntent: Intent) { |
There was a problem hiding this comment.
This uses launcherIntent instead of the launcherBaseIntent that is used in the interface. This leads to a warning in the IDE ("This may cause problems when calling this function with named arguments").
| return | ||
| } | ||
|
|
||
| showDialogRunnable = createShowDialogRunnable(activity, launcherIntent) |
There was a problem hiding this comment.
This should be a local variable. That way we can get rid of the !! and we don't leak the Activity when ´FundingReminderholds on to it via theshowDialogRunnable` property.
| .setPositiveButton(R.string.funding_googleplay_contribution_reminder_positive_button) { _, _ -> | ||
| activity.startActivity(getSupportIntent(launcherIntent)) | ||
| } | ||
| .setNegativeButton(R.string.funding_googleplay_contribution_reminder_negative_button) { _, _ -> }.show() |
There was a problem hiding this comment.
| .setNegativeButton(R.string.funding_googleplay_contribution_reminder_negative_button) { _, _ -> }.show() | |
| .setNegativeButton(R.string.funding_googleplay_contribution_reminder_negative_button, null) | |
| .show() |
| @@ -90,4 +90,9 @@ | |||
| <item>Make our boldest ideas a reality</item> | |||
| <item>Contributions are not tax-deductible</item> | |||
| </array> | |||
|
|
|||
| <string name="funding_googleplay_contribution_reminder_title">Support Thunderbird</string> | |||
There was a problem hiding this comment.
We could reuse funding_googleplay_contribution_header_title for this.
| @@ -33,7 +33,7 @@ class Settings { | |||
| * | |||
| * @see SettingsExporter | |||
| */ | |||
| public static final int VERSION = 100; | |||
| public static final int VERSION = 101; | |||
There was a problem hiding this comment.
This is the version for settings export. If we want the value in the settings file, an entry has to be added to GeneralSettingsDescriptions.
There was a problem hiding this comment.
The settings need to be improved, it's unclear which component belongs to what functionality and what defines that a setting is importable/exportable.
| override fun getLong(key: String, defValue: Long): Long { | ||
| return when (key) { | ||
| "fundingReminderShownTimestamp" -> K9.fundingReminderShownTimestamp | ||
| else -> defValue | ||
| } | ||
| } | ||
|
|
||
| override fun putLong(key: String?, value: Long) { | ||
| when (key) { | ||
| "fundingReminderShownTimestamp" -> K9.fundingReminderShownTimestamp = value | ||
| else -> return | ||
| } | ||
|
|
||
| saveSettings() | ||
| } |
There was a problem hiding this comment.
GeneralSettingsDataStore is only used by the settings screen. Since we don't have a visual setting, this is not necessary.
There was a problem hiding this comment.
It would be good to document how this is supposed to work. From reading the implementation, it's not obvious.
|
Closing this in favor of #8397 and a reworked reminder logic. |
This adds the forgotten funding reminder. It will show once after 30 minutes of activity.