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

WIP Add support for only rendering certain sub-trees. #63

Closed
wants to merge 1 commit into from

Conversation

zach-klippenstein
Copy link
Collaborator

@zach-klippenstein zach-klippenstein commented Aug 23, 2020

This change allows filters to be written which not only exclude certain subtrees, but also which only include certain subtrees.

It has three parts:

  1. Change ViewFilter's matches method to return a three-value type instead of a boolean, where the third value means don't include this view, but include its children.
  2. Give ComposeLayoutInfo a pointer to its parent.
  3. Write a filter for compose that returns "include only children" for all nodes that don't have a parent which matches a certain test tag.

Open questions

  • Is the new ViewFilter API acceptable?
  • Should viewFilterFor still accept a boolean lambda instead of all 3 values, since the third value is likely not needed by most filters and it would allow more readable single-expression filters.
  • Is searching up the tree on every node an acceptable cost?
  • This approach requires the filter to be aware of all types that are visitable - a composable nested in an Android view nested inside a composable would also need to walk up the inner View hierarchy so that it included the complete subtrees. Would a better approach to make the filter stateful, so it could set an internal flag while visiting children that match? This would also be more efficient, since it wouldn't need to walk up the parent chain for every node. However it requires a slightly bigger change to the filter API.

One way to solve the last point would be to pass an arbitrary value down the filter chain:

fun interface ViewFilter<S> {
  fun matches(view: Any, filterState: S): Pair<FilterResult, S>
}

fun startFromTestTag(testTag: String): ViewFilter =
  viewFilterFor<ComposeLayoutInfo> { layoutInfo, alreadyMatched ->
    if (alreadyMatched || (testTag in testTags)) {
      Pair(INCLUDE, true)
    } else {
      Pair(INCLUDE_ONLY_CHILDREN, false)
    }
  }

We could also then provide two versions of viewFilterFor, a simple one (no filter state, boolean return) and a complete one (same signature as the above method). One issue with this is that the returned filter state value would be ignored if the result is EXCLUDE, so a sealed class might be better for the result than an enum at that point:

sealed class FilterResult<out S> {
  class Include<S>(val filterState: S) : FilterResult<S>()
  class IncludeOnlyChildren<S>(val filterState: S) : FilterResult<S>()
  object Exclude : FilterResult<Nothing>()
}

Fixes #58.

@zach-klippenstein zach-klippenstein force-pushed the zachklipp/tree-visiting-selectors branch from c019c9e to 76f2fde Compare August 23, 2020 16:03
radiography/src/main/java/radiography/RenderTreeString.kt Outdated Show resolved Hide resolved
radiography/src/main/java/radiography/RenderTreeString.kt Outdated Show resolved Hide resolved
// Child may be null, if children were removed by another thread after we captured the child
// count. getChildAt returns null for invalid indices, it doesn't throw.
val child = view.getChildAt(index) ?: continue
when (viewFilter.matches(child)) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This filter+add logic is duplicated, factor out an addFilteredChild function?

@zach-klippenstein zach-klippenstein force-pushed the zachklipp/tree-visiting-selectors branch from 76f2fde to 5c9afac Compare August 27, 2020 16:25
@zach-klippenstein
Copy link
Collaborator Author

Rethinking the whole codebase, might be replaced by this branch.

@zach-klippenstein
Copy link
Collaborator Author

This is being done a different and much better way in #70.

@zach-klippenstein zach-klippenstein deleted the zachklipp/tree-visiting-selectors branch September 1, 2020 19:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Drop SlotTable.scan, add composable selection support
1 participant