[android] solves issue of status change of "search history" button on… #8087

2024-05-05T12:15:22Z

kavikhalique commented

2024-05-05 12:15:22 +00:00

(Migrated from github.com)

Fixes #8078

Solves the issue of showing changes after even taps on search History button inside settings.

Fixes #8078 Solves the issue of showing changes after even taps on search History button inside settings.

👍 1

biodranik (Migrated from github.com) reviewed 2024-05-05 12:24:39 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

biodranik (Migrated from github.com) commented

2024-05-05 12:24:39 +00:00

Will this work?

    pref.setOnPreferenceChangeListener((preference, Boolean newValue) -> {
      if (newValue != Config.isSearchHistoryEnabled())

Will this work? ``` pref.setOnPreferenceChangeListener((preference, Boolean newValue) -> { if (newValue != Config.isSearchHistoryEnabled()) ```

kavikhalique (Migrated from github.com) reviewed 2024-05-05 12:26:31 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

kavikhalique (Migrated from github.com) commented

2024-05-05 12:26:31 +00:00

Yes ofcourse but i did that to keep it in proper format

biodranik (Migrated from github.com) reviewed 2024-05-05 12:27:39 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

biodranik (Migrated from github.com) commented

2024-05-05 12:27:39 +00:00

What is "proper format"?

kavikhalique (Migrated from github.com) reviewed 2024-05-05 12:32:31 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

kavikhalique (Migrated from github.com) commented

2024-05-05 12:32:31 +00:00

Like declaring a variable first storing value into that and then using it for further :)

biodranik (Migrated from github.com) reviewed 2024-05-05 13:12:22 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

biodranik (Migrated from github.com) commented

2024-05-05 13:12:22 +00:00

So you likely meant "a more complex code to read", right?

kavikhalique (Migrated from github.com) reviewed 2024-05-05 14:10:02 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

kavikhalique (Migrated from github.com) commented

2024-05-05 14:10:01 +00:00

Yes, but easy to understand.
But since you suggested so i have changed the code accordingly : )

Yes, but easy to understand. But since you suggested so i have changed the code accordingly : )

biodranik (Migrated from github.com) reviewed 2024-05-05 14:41:32 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

biodranik (Migrated from github.com) commented

2024-05-05 14:41:31 +00:00

My suggestion was one line shorter )

kavikhalique (Migrated from github.com) reviewed 2024-05-05 21:35:40 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

kavikhalique (Migrated from github.com) commented

2024-05-05 21:35:40 +00:00

Yes i did that :)

biodranik (Migrated from github.com) reviewed 2024-05-05 22:31:03 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

					
				@ -258,7 +258,7 @@ public class SettingsPrefsFragment extends BaseXmlSettingsFragment

				    ((TwoStatePreference) pref).setChecked(isHistoryEnabled);

				    pref.setOnPreferenceChangeListener((preference, newValue) -> {

				      boolean newVal = (Boolean) newValue;

biodranik (Migrated from github.com) commented

2024-05-05 22:31:03 +00:00

    pref.setOnPreferenceChangeListener((preference, Boolean newValue) -> {

```suggestion pref.setOnPreferenceChangeListener((preference, Boolean newValue) -> { ```

kavikhalique (Migrated from github.com) reviewed 2024-05-05 22:38:55 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

kavikhalique (Migrated from github.com) commented

2024-05-05 22:38:54 +00:00

My suggestion was one line shorter )

I missed that another line that time. Since it was pre written code so i pre assumed that i will not be asked to delete the pre written codes : )

> My suggestion was one line shorter ) I missed that another line that time. Since it was pre written code so i pre assumed that i will not be asked to delete the pre written codes : )

kavikhalique (Migrated from github.com) reviewed 2024-05-05 22:58:40 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

					
				@ -258,7 +258,7 @@ public class SettingsPrefsFragment extends BaseXmlSettingsFragment

				    ((TwoStatePreference) pref).setChecked(isHistoryEnabled);

				    pref.setOnPreferenceChangeListener((preference, newValue) -> {

				      boolean newVal = (Boolean) newValue;

kavikhalique (Migrated from github.com) commented

2024-05-05 22:58:40 +00:00

If we remove (newVal) variable then we need to refactor it everywhere because it has been used later also and everywhere we need to type cast it to boolean will it be ok?

I am committing it please review it.

If we remove (newVal) variable then we need to refactor it everywhere because it has been used later also and everywhere we need to type cast it to boolean will it be ok? I am committing it please review it.

biodranik (Migrated from github.com) reviewed 2024-05-06 12:20:49 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

					
				@ -258,7 +258,7 @@ public class SettingsPrefsFragment extends BaseXmlSettingsFragment

				    ((TwoStatePreference) pref).setChecked(isHistoryEnabled);

biodranik (Migrated from github.com) commented

2024-05-06 12:20:49 +00:00

    pref.setOnPreferenceChangeListener((preference, Boolean newValue) -> {

Will it compile? Then you don't need a (Boolean) cast below.

```suggestion pref.setOnPreferenceChangeListener((preference, Boolean newValue) -> { ``` Will it compile? Then you don't need a (Boolean) cast below.

kavikhalique (Migrated from github.com) reviewed 2024-05-06 21:17:47 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

					
				@ -258,7 +258,7 @@ public class SettingsPrefsFragment extends BaseXmlSettingsFragment

				    ((TwoStatePreference) pref).setChecked(isHistoryEnabled);

kavikhalique (Migrated from github.com) commented

2024-05-06 21:17:47 +00:00

No we cannot do that because its required data type is of type "Object".

biodranik (Migrated from github.com) approved these changes 2024-05-06 21:46:34 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

					
				@ -258,7 +258,7 @@ public class SettingsPrefsFragment extends BaseXmlSettingsFragment

				    ((TwoStatePreference) pref).setChecked(isHistoryEnabled);

biodranik (Migrated from github.com) commented

2024-05-06 21:44:44 +00:00

I see, poor Java... Then your solution with an intermediate casted variable is cleaner, you were right )

kavikhalique (Migrated from github.com) reviewed 2024-05-06 21:58:13 +00:00

android/app/src/main/java/app/organicmaps/settings/SettingsPrefsFragment.java

					
				@ -258,7 +258,7 @@ public class SettingsPrefsFragment extends BaseXmlSettingsFragment

				    ((TwoStatePreference) pref).setChecked(isHistoryEnabled);