organicmaps/organicmaps-tmp
Archived
13
Fork 1
Code Issues 2.7k Pull requests 182 Projects Releases 65 Wiki Activity Actions

[android] Fix bookmark edits not updating in place page on screen rotation #10260

Open
savsch wants to merge 1 commit from savsch/fix/android-pp-bookmark-edit-issue2418 into master
pull from: savsch/fix/android-pp-bookmark-edit-issue2418
merge into: organicmaps:master
Conversation 0 Commits 1 Files changed 2 +9 -1
2 changed files with 9 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
android/app/src/main/java/app/organicmaps/widget/placepage/EditBookmarkFragment.java
View file

@ -181,6 +181,14 @@ public class EditBookmarkFragment extends BaseMwmDialogFragment implements View.
}
}
@Override
public void onAttach(@NonNull Context context)
{
super.onAttach(context);
if (mListener == null && getParentFragment() instanceof EditBookmarkListener)
pastk commented 2025-02-22 12:08:34 +00:00
Review
Copy link

Could you please add a comment explaining why this is necessary.

Also I wonder why there is no issue with "lost" listener when a parent is BookmarksListFragment

Could you please add a comment explaining why this is necessary. Also I wonder why there is no issue with "lost" listener when a parent is BookmarksListFragment
savsch commented 2025-03-01 22:31:21 +00:00 (Migrated from github.com)
Review
Copy link

Sorry to keep you waiting; been very busy the last few days.

why there is no issue with "lost" listener when a parent is BookmarksListFragment

BookmarkListActivity has a configChanges="orientation|..." unlike MwmActivity where the issue occurs.

Also, now that I think about it a bit more, I shouldn't have resorted to the onAttach workaround.

There's an arguably cleaner way to fix this:

PlacePageBookmarkFragment.java
@@ -19,6 +19,7 @@ import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.fragment.app.Fragment;
 import androidx.fragment.app.FragmentActivity;
+import androidx.fragment.app.FragmentFactory;
 import androidx.lifecycle.Observer;
 import androidx.lifecycle.ViewModelProvider;
 import app.organicmaps.R;
@@ -120,6 +121,25 @@ public class PlacePageBookmarkFragment extends Fragment implements View.OnClickL
     }
   }
 
+  @Override
+  public void onCreate(@Nullable Bundle savedInstanceState)
+  {
+    // Custom FragmentFactory to prevent EditBookmarkFragment's mListener
+    // from becoming null on configuration changes
+    getChildFragmentManager().setFragmentFactory(new FragmentFactory()
+    {
+      @NonNull
+      @Override
+      public Fragment instantiate(@NonNull ClassLoader classLoader, @NonNull String className)
+      {
+        Class<? extends Fragment> fragmentClass = loadFragmentClass(classLoader, className);
+        Fragment fragment = super.instantiate(classLoader, className);
+        if (fragmentClass == EditBookmarkFragment.class)
+          ((EditBookmarkFragment) fragment).setEditBookmarkListener(PlacePageBookmarkFragment.this);
+        return fragment;
+      }
+    });
+    super.onCreate(savedInstanceState);
+  }
+
   @Override
   public void onClick(View v)
   {
@@ -127,7 +147,7 @@ public class PlacePageBookmarkFragment extends Fragment implements View.OnClickL
     EditBookmarkFragment.editBookmark(currentBookmark.getCategoryId(),
                                       currentBookmark.getBookmarkId(),
                                       activity,
-                                      activity.getSupportFragmentManager(),
+                                      getChildFragmentManager(),
                                       PlacePageBookmarkFragment.this);
   }

Should I push this instead?

Sorry to keep you waiting; been very busy the last few days. --- > why there is no issue with "lost" listener when a parent is BookmarksListFragment `BookmarkListActivity` has a `configChanges="orientation|..."` unlike `MwmActivity` where the issue occurs. --- Also, now that I think about it a bit more, I shouldn't have resorted to the `onAttach` workaround. There's an arguably cleaner way to fix this: ```diff PlacePageBookmarkFragment.java @@ -19,6 +19,7 @@ import androidx.annotation.NonNull; import androidx.annotation.Nullable; import androidx.fragment.app.Fragment; import androidx.fragment.app.FragmentActivity; +import androidx.fragment.app.FragmentFactory; import androidx.lifecycle.Observer; import androidx.lifecycle.ViewModelProvider; import app.organicmaps.R; @@ -120,6 +121,25 @@ public class PlacePageBookmarkFragment extends Fragment implements View.OnClickL } } + @Override + public void onCreate(@Nullable Bundle savedInstanceState) + { + // Custom FragmentFactory to prevent EditBookmarkFragment's mListener + // from becoming null on configuration changes + getChildFragmentManager().setFragmentFactory(new FragmentFactory() + { + @NonNull + @Override + public Fragment instantiate(@NonNull ClassLoader classLoader, @NonNull String className) + { + Class<? extends Fragment> fragmentClass = loadFragmentClass(classLoader, className); + Fragment fragment = super.instantiate(classLoader, className); + if (fragmentClass == EditBookmarkFragment.class) + ((EditBookmarkFragment) fragment).setEditBookmarkListener(PlacePageBookmarkFragment.this); + return fragment; + } + }); + super.onCreate(savedInstanceState); + } + @Override public void onClick(View v) { @@ -127,7 +147,7 @@ public class PlacePageBookmarkFragment extends Fragment implements View.OnClickL EditBookmarkFragment.editBookmark(currentBookmark.getCategoryId(), currentBookmark.getBookmarkId(), activity, - activity.getSupportFragmentManager(), + getChildFragmentManager(), PlacePageBookmarkFragment.this); } ``` Should I push this instead?
pastk commented 2025-03-02 05:09:31 +00:00
Review
Copy link

IDK, it looks more complex for sure :)

It'd be great for more knowledgeable @organicmaps/android folks to have a look

IDK, it looks more complex for sure :) It'd be great for more knowledgeable @organicmaps/android folks to have a look
mListener = (EditBookmarkListener) getParentFragment();
}
private void initToolbar(View view)
{
Toolbar toolbar = view.findViewById(R.id.toolbar);

2
android/app/src/main/java/app/organicmaps/widget/placepage/sections/PlacePageBookmarkFragment.java
View file

@ -127,7 +127,7 @@ public class PlacePageBookmarkFragment extends Fragment implements View.OnClickL
EditBookmarkFragment.editBookmark(currentBookmark.getCategoryId(),
currentBookmark.getBookmarkId(),
activity,
activity.getSupportFragmentManager(),
getChildFragmentManager(),
PlacePageBookmarkFragment.this);
}