search

thunderbird / thunderbird-android

Thunderbird for Android – Open Source Email App for Android (fka K-9 Mail)
https://thunderbird.net/mobile
Apache License 2.0
11.05k stars 2.51k forks source link

Disable Unified Inbox if there's only one account #8459

Closed shamim-emon closed 2 weeks ago

shamim-emon commented 3 weeks ago

Fixes #7556

shamim-emon commented 3 weeks ago

@cketti @wmontwe could you please take a look and let me know if any feedback. Thanks in advance

shamim-emon commented 2 weeks ago

I suggest moving the logic for controlling isShowUnifiedInbox to com.fsck.k9.account.AccountCreator. Since the account is persisted after a successful call to AccountCreator.create(), this is the ideal place to handle this setting.

@wmontwe Done!

shamim-emon commented 2 weeks ago

Why is it necessary to use a dedicated PreferenceDataStore for these settings instead of modifying K9's settings directly?

Also, please combine your commits into a single one. The current structure is fine otherwise.

@wmontwe got rid of the dedicated PreferenceDataStoreand modifying K9's settings directly

cketti commented 2 weeks ago

When changing the default value of a setting, this should be reflected in the associated "settings description". In this case: https://github.com/thunderbird/thunderbird-android/blob/2c550badeda212c8cab4696b16df58830c60b660/legacy/core/src/main/java/com/fsck/k9/preferences/GeneralSettingsDescriptions.java#L173-L175

Steps that need to be performed:

  1. Increment Settings.VERSION
  2. Add a line to the showUnifiedInbox entry in GeneralSettingsDescriptions using the new version number and default value.

Unfortunately, the QR code import from Thunderbird desktop currently doesn't use AccountCreator. So even if users import more than one account from Thunderbird desktop, the unified inbox will stay disabled. I don't think this has to be fixed right away. But we should create an issue once this PR is merged to keep track of this problem.

shamim-emon commented 2 weeks ago

When changing the default value of a setting, this should be reflected in the associated "settings description". In this case:

https://github.com/thunderbird/thunderbird-android/blob/2c550badeda212c8cab4696b16df58830c60b660/legacy/core/src/main/java/com/fsck/k9/preferences/GeneralSettingsDescriptions.java#L173-L175

Steps that need to be performed:

  1. Increment Settings.VERSION
  2. Add a line to the showUnifiedInbox entry in GeneralSettingsDescriptions using the new version number and default value.

Unfortunately, the QR code import from Thunderbird desktop currently doesn't use AccountCreator. So even if users import more than one account from Thunderbird desktop, the unified inbox will stay disabled. I don't think this has to be fixed right away. But we should create an issue once this PR is merged to keep track of this problem.

@cketti done, hope I got #2 right? Also, I will create an issue for Thunderbird desktop once this PR is merged.

cketti commented 2 weeks ago

@shamim-emon: Please undo the unrelated formatting changes. They bury the actual changes.

shamim-emon commented 2 weeks ago

@shamim-emon: Please undo the unrelated formatting changes. They bury the actual changes.

@cketti done!

shamim-emon commented 2 weeks ago

@cketti @wmontwe please let me know if anymore changes required and/ or if I have missed anything. Thanks!

wmontwe commented 2 weeks ago

Added issue to track missing QR code changes: #8531

@shamim-emon Could you please squash your changes into one commit and update this pull-request. Then it's ready.

shamim-emon commented 2 weeks ago

Added issue to track missing QR code changes: #8531

@shamim-emon Could you please squash your changes into one commit and update this pull-request. Then it's ready.

@wmontwe thanks for creating & sharing the issue.

Also I have squashed commits into one. Please have a look and let me know if anything else needs to be done from my side. Thanks