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.

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

Sub-children expansion not expanded on updateDataSet #389

Closed
gwailoTr0n5000 opened this issue Jun 20, 2017 · 7 comments
Closed

Sub-children expansion not expanded on updateDataSet #389

gwailoTr0n5000 opened this issue Jun 20, 2017 · 7 comments

Comments

@gwailoTr0n5000
Copy link
Contributor

gwailoTr0n5000 commented Jun 20, 2017

Hello David, I hope you're doing well. Really appreciate the recent changes and update! Thank you.

I've been working on a particular bug and can't seem to figure out how to get it sorted out. We have relatively deep recursion/expansion in our lists and are using your multi-level expansion to support this in our RecyclerViews. This has been going well except for getting sub children whose data model saves the expansion state to expand on startup. If they're top level they expand as expected, but if they are in deeper levels they do not initially expand. I've tried many different parameters/methods including the expandAtStartup() call but none seem to work. Is this just not supported? If not, what would you suggest as the cleanest implementation?

Thank you!
Current customization code:

inAdapter
    .setHandleDragEnabled(true) //Enable handle drag (handle view must be set in the VH)
    .setSwipeEnabled(true) //Enable swipe items
    .setAutoCollapseOnExpand(false);

inAdapter.setHandleDragEnabled(false);
inAdapter.setAutoCollapseOnExpand(false);
inAdapter.setMode(SelectableAdapter.Mode.MULTI);
inAdapter.expandItemsAtStartUp();
@gwailoTr0n5000
Copy link
Contributor Author

gwailoTr0n5000 commented Jun 20, 2017

Went digging around in the code again and I think I found the problem:

Jump to code in FlexibleAdapter

private void prepareItemsForUpdate(List<T> newItems) {
	// Display Scrollable Headers and Footers
	restoreScrollableHeadersAndFooters(newItems);

		int position = 0;
	IHeader sameHeader = null;
	// We use 1 cycle for expanding And display headers
	// to optimize the operations of adding hidden items/subItems
	while (position < newItems.size()) {
		T item = newItems.get(position);
		// Expand Expandable
		if (isExpanded(item)) {
			IExpandable expandable = (IExpandable) item;
			expandable.setExpanded(true);
			List<T> subItems = getExpandableList(expandable);
			int itemCount = newItems.size();
			if (position < itemCount) {
				newItems.addAll(position + 1, subItems);
				position += subItems.size();
			} else {
				newItems.addAll(subItems);
				position = itemCount;
			}
		}
		// Display headers too
		if (!headersShown && isHeader(item) && !item.isHidden()) {
			headersShown = true;
		}
		IHeader header = getHeaderOf(item);
		if (header != null && !header.equals(sameHeader) && !isExpandable((T) header)) {
			header.setHidden(false);
			sameHeader = header;
			newItems.add(position, (T) header);
			position++;
		}
		position++;
	}
}

When position += subItems.size() you jump over all the subitems without handling their expansion status. I'm not sure if this was intentional but it would be great to make this at least tied to a parameter on depth to auto expand initially. I'm going to try my hand at coding it up and can send you a pull request if you'd like as we really need this feature.

Any input is much appreciated.

Cheers

@davideas
Copy link
Owner

davideas commented Jun 20, 2017

@gwailoTr0n5000, thank you for compliments.

Regarding the startup with deep expansion, it should already work if you use the constructor, because it uses the expand method with "init" mode called in expandItemsAtStartUp().

That part of the code you posted is for updateDataSet only, and that was indeed intentional. But we can always adapt it.

@gwailoTr0n5000
Copy link
Contributor Author

Gotcha, so our use case is after the object is created we load from server and in the case it's a significant change we were using the update code to pass the entire set in. It sounds like I should model my changes on the expandItemsAtStartUp() code then, correct? Would you want a parameter to govern the update behavior?

I tried to hack around the issue with an immediate call to the expandItemsAtStartUp() method after performing the update but end up getting doubling of the subitems since the top level items are already expanded during the update.

temp_adapter.updateDataSet(inParsedIntermediate.getStateList()); temp_adapter.expandItemsAtStartUp();

@davideas
Copy link
Owner

davideas commented Jun 20, 2017

@gwailoTr0n5000, you can try to override the function expandItemsAtStartUp(), however the best is to modify the update, by removing the 2 commented lines:

if (position < itemCount) {
	newItems.addAll(position + 1, subItems);
	//position += subItems.size();
} else {
	newItems.addAll(subItems);
	//position = itemCount;
}

If you want you can participate to it with a pull request.
The change should not affect performance since the outer if statement will skip all the subItems not expandable. For who has multilevel expandable, that is a feature that might be needed.

However, that need to be published: I might create a snapshot version with this modification, but need to wait some days, since the snapshot is still used in many others projects that will be affected by the publication that will include RC2 as well that will not make compile anymore. By when you need this?
Note: Be aware that if you use snapshot, you will receive live updates from my all new publication without advice!

@davideas davideas changed the title Sub-children expansion at startup not triggering Sub-children expansion not expanded on updateDataSet Jun 20, 2017
@davideas davideas added the bug label Jun 20, 2017
@gwailoTr0n5000
Copy link
Contributor Author

gwailoTr0n5000 commented Jun 21, 2017

Happy to do the pull request and send that up but I'm having trouble using the local version with my code to test and verify. When I pull it in as a module I end up with an error I haven't been able to sort out yet:

"Error:Project : declares a dependency from configuration 'compile' to configuration 'default' which is not declared in the descriptor for project :libs:FlexibleAdapter."

Project will compile and run independently, though.

P.S. The lack of nested expansion is currently our top bug, so either getting it working from the local module for now or getting a snapshot or other way to access and pull it in is pretty critical for us at the moment. Thanks for all your help!

@gwailoTr0n5000
Copy link
Contributor Author

Figured out a way to import by building the .aar and then importing that. (Android Studio gave me a lot of trouble with the many other methods I tried of importing a module.) The two removed lines solves the bug for multi-level expansion on update. I've already created the pull request. Please let me know if there's anything else I can do to help out!

@davideas
Copy link
Owner

@gwailoTr0n5000 , thanks.
It will be published with RC3 release and next snapshot.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants