Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

sub-detail fixes #2303

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

sub-detail fixes #2303

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1f24211
Retains the list on rotation
shubham1g5 Aug 1, 2020
aeae548
Fixes xpath crashes happening due to user moving out of the sub detai…
shubham1g5 Aug 1, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 26 additions & 13 deletions app/src/org/commcare/fragments/EntitySubnodeDetailFragment.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import android.app.Activity;
import android.os.Bundle;
import android.util.Log;
import android.view.LayoutInflater;
import android.view.View;
import android.view.ViewGroup;
Expand All @@ -22,6 +23,8 @@

import java.util.List;

import androidx.annotation.Nullable;

/**
* Created by jschweers on 8/26/2015.
* <p/>
Expand All @@ -35,6 +38,12 @@ public class EntitySubnodeDetailFragment extends EntityDetailFragment implements
public EntitySubnodeDetailFragment() {
}

@Override
public void onCreate(@Nullable Bundle savedInstanceState) {
super.onCreate(savedInstanceState);
setRetainInstance(true);
}

@Override
public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle savedInstanceState) {
if (savedInstanceState != null) {
Expand Down Expand Up @@ -65,6 +74,8 @@ public View onCreateView(LayoutInflater inflater, ViewGroup container, Bundle sa
headerLayout.removeAllViews();
headerLayout.addView(headerView);
headerLayout.setVisibility(View.VISIBLE);
} else if (adapter != null) {
listView.setAdapter((ListAdapter)adapter);
}

return rootView;
Expand All @@ -79,20 +90,22 @@ public void attachLoader(EntityLoaderTask task) {
public void deliverLoadResult(List<Entity<TreeReference>> entities,
List<TreeReference> references,
NodeEntityFactory factory, int focusTargetIndex) {
Bundle args = getArguments();
Detail detail = asw.getSession().getDetail(args.getString(DETAIL_ID));
final int thisIndex = args.getInt(CHILD_DETAIL_INDEX, -1);
final boolean detailCompound = thisIndex != -1;
if (detailCompound) {
detail = getChildDetailForDisplay(detail, thisIndex);
}
if(isVisible()) { // underlying session might have changed otherwise and can cause xpath eval errors
Bundle args = getArguments();
Detail detail = asw.getSession().getDetail(args.getString(DETAIL_ID));
final int thisIndex = args.getInt(CHILD_DETAIL_INDEX, -1);
final boolean detailCompound = thisIndex != -1;
if (detailCompound) {
detail = getChildDetailForDisplay(detail, thisIndex);
}

this.loader = null;
this.adapter = new EntitySubnodeDetailAdapter(getActivity(), detail, references,
entities, modifier, factory);
this.listView.setAdapter((ListAdapter)this.adapter);
if (focusTargetIndex != -1) {
listView.setSelection(focusTargetIndex);
this.loader = null;
this.adapter = new EntitySubnodeDetailAdapter(getActivity(), detail, references,
entities, modifier, factory);
this.listView.setAdapter((ListAdapter)this.adapter);
if (focusTargetIndex != -1) {
listView.setSelection(focusTargetIndex);
}
}
}

Expand Down
4 changes: 1 addition & 3 deletions app/src/org/commcare/views/TabbedDetailView.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,9 @@ public void setRoot(ViewGroup root) {

mMenu = root.findViewById(R.id.tabbed_detail_menu);
mViewPager = root.findViewById(R.id.tabbed_detail_pager);
mViewPager.setId(AndroidUtil.generateViewId());
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this already has an id set in xml and setting dynamic ids result in fragment not getting re-attached to the activity.

Copy link
Member

Choose a reason for hiding this comment

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

Good catch. It seems really strange that the original line was set at all, do you have a sense of what was trying to be accomplished with it? The only reason I can think you'd set the ID manually would be if you intended to have two ViewPagers within the same Activity.

I went back to check on the git blames and it does look like that is explicitly why that change was made:
d2eab7c

The only context where I can think this occurs is with persistent case tiles which do have their own view pager. The previous commit from the one I linked above is

a5b7f23

which does discuss persistent case tiles, and it's in the persistent case tile PR
#81

I think the key thing to test is loading a case list screen with a view pager, along with a persistent case tile that has its own view pager. I think both end up in the same general view hierarchy, so they need distinct ID's just based on how the views work but it's possible that other refactors have made that unecessary.


mViewPageTabStrip = root.findViewById(R.id.pager_tab_strip);

mViewPager.setOnPageChangeListener(new ViewPager.OnPageChangeListener() {
mViewPager.addOnPageChangeListener(new ViewPager.OnPageChangeListener() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no-op change since setOnPageChangeListener is deprecated.


@Override
public void onPageSelected(int position) {
Expand Down