Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -338,7 +338,6 @@ class ComposeUiTest {
@Suppress("RemoveCurlyBracesFromTemplate")
assertThat(hierarchy).contains(
"""
Box:
$BLANK ${BLANK}Box { test-tag:"root" }
$BLANK ${BLANK}├─Box
$BLANK ${BLANK}├─Column
Expand Down Expand Up @@ -393,7 +392,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}╰─Dialog
|${BLANK} ╰─CompositionLocalProvider { DIALOG }
Expand Down Expand Up @@ -422,7 +420,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}╰─CustomTestDialog
|${BLANK} ╰─CompositionLocalProvider { DIALOG }
Expand All @@ -447,7 +444,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}╰─SingleSubcompositionLayout { test-tag:"subcompose-layout" }
|${BLANK} ╰─<subcomposition of SingleSubcompositionLayout>
Expand All @@ -473,7 +469,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}╰─SingleSubcompositionLayout { test-tag:"subcompose-layout" }
|${BLANK} ╰─<subcomposition of SingleSubcompositionLayout>
Expand Down Expand Up @@ -505,7 +500,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}╰─MultipleSubcompositionLayout { test-tag:"subcompose-layout" }
|${BLANK} ├─<subcomposition of MultipleSubcompositionLayout>
Expand Down Expand Up @@ -537,7 +531,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}├─SingleSubcompositionLayout { test-tag:"subcompose-layout1" }
|${BLANK}│ ╰─<subcomposition of SingleSubcompositionLayout>
Expand Down Expand Up @@ -565,7 +558,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}╰─BoxWithConstraints { test-tag:"with-constraints" }
|${BLANK} ╰─<subcomposition of BoxWithConstraints>
Expand Down Expand Up @@ -595,7 +587,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { test-tag:"parent" }
|${BLANK}╰─LazyColumn { test-tag:"list" }
|${BLANK} ├─<subcomposition of LazyColumn>
Expand Down Expand Up @@ -646,7 +637,6 @@ class ComposeUiTest {

assertThat(hierarchy).isEqualTo(
"""
|CompositionLocalProvider:
|${BLANK}CompositionLocalProvider { 10×30px, test-tag:"parent" }
|${BLANK}╰─MultipleSubcompositionLayout { 10×30px, test-tag:"subcompose-layout" }
|${BLANK} ├─<subcomposition of MultipleSubcompositionLayout>
Expand Down
2 changes: 1 addition & 1 deletion radiography/api/radiography.api
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ public final class radiography/ScanScopes {
public static final fun singleViewScope (Landroid/view/View;)Lradiography/ScanScope;
}

public abstract class radiography/ScannableView {
public abstract interface class radiography/ScannableView {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Also worth noting this break backward compatibility (as you can't switch over ScannableView anymore) but I don't expect any consumer to be impacted.

If we really really want to avoid that we could keep ScannableView as abstract class and make it implement an interface. But then I don't know how to name that interface

Copy link
Copy Markdown
Collaborator

@zach-klippenstein zach-klippenstein Mar 3, 2021

Choose a reason for hiding this comment

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

I don't think it's possible to add direct subclasses to a sealed class in any way without technically breaking compatibility. Any consuming bytecode that assumes it has handled all the cases will be wrong.

Technically I think this means we need to bump the major version number.

public abstract fun getChildren ()Lkotlin/sequences/Sequence;
public abstract fun getDisplayName ()Ljava/lang/String;
}
Expand Down
39 changes: 4 additions & 35 deletions radiography/src/main/java/radiography/Radiography.kt
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
package radiography

import android.os.Handler
import android.os.Looper
import android.view.View
import android.view.WindowManager
import androidx.annotation.VisibleForTesting
import radiography.Radiography.scan
import radiography.ScanScopes.AllWindowsScope
Expand All @@ -15,17 +11,15 @@ import java.util.concurrent.TimeUnit.SECONDS

/**
* Utility class to scan through a view hierarchy and pretty print it to a [String].
* Call [scan] or [View.scan].
* Call [scan] or [android.view.View.scan].
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: the kotlin formatter should keep the import even if it's only used by kdoc.

*/
public object Radiography {

/**
* Scans the view hierarchies and pretty print them to a [String].
*
* You should generally call this method from the main thread, as views are meant to be accessed
* from a single thread. If you call this from a background thread, this will schedule a message
* to the main thread to retrieve the view hierarchy from there and will wait up to 5 seconds
* or return an error message. This method will never throw, any thrown exception will have
* from a single thread. This method will never throw, any thrown exception will have
* its message included in the returned string.
*
* @param scanScope the [ScanScope] that determines what to scan. [AllWindowsScope] by default.
Expand All @@ -52,27 +46,11 @@ public object Radiography {
}

roots.forEach { scanRoot ->
// The entire view tree is single threaded, and that's typically the main thread, but
// it doesn't have to be, and we don't know where the passed in view is coming from.
val viewLooper = (scanRoot as? AndroidView)?.view?.handler?.looper
?: Looper.getMainLooper()!!

if (viewLooper.thread == Thread.currentThread()) {
scanFromLooperThread(scanRoot, viewStateRenderers, viewFilter)
} else {
val latch = CountDownLatch(1)
Handler(viewLooper).post {
scanFromLooperThread(scanRoot, viewStateRenderers, viewFilter)
latch.countDown()
}
if (!latch.await(5, SECONDS)) {
return "Could not retrieve view hierarchy from main thread after 5 seconds wait"
}
}
scanSingleRoot(scanRoot, viewStateRenderers, viewFilter)
}
}

private fun StringBuilder.scanFromLooperThread(
private fun StringBuilder.scanSingleRoot(
rootView: ScannableView,
viewStateRenderers: List<ViewStateRenderer>,
viewFilter: ViewFilter
Expand All @@ -83,17 +61,8 @@ public object Radiography {
appendLine()
}

val androidView = (rootView as? AndroidView)?.view
val layoutParams = androidView?.layoutParams
val title = (layoutParams as? WindowManager.LayoutParams)?.title?.toString()
?: rootView.displayName
appendLine("$title:")

val startPosition = length
try {
androidView?.let {
appendLine("window-focus:${it.hasWindowFocus()}")
}
renderScannableViewTree(this, rootView, viewStateRenderers, viewFilter)
} catch (e: Throwable) {
insert(
Expand Down
23 changes: 16 additions & 7 deletions radiography/src/main/java/radiography/ScannableView.kt
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package radiography

import android.view.View
import android.view.ViewGroup
import android.view.WindowManager
import androidx.compose.ui.Modifier
import radiography.ScannableView.AndroidView
import radiography.ScannableView.ComposeView
Expand All @@ -18,16 +19,24 @@ import radiography.internal.mightBeComposeView
* Can either be an actual Android [View] ([AndroidView]) or a grouping of Composables that roughly
* represents the concept of a logical "view" ([ComposeView]).
*/
public sealed class ScannableView {
public interface ScannableView {

/** The string that be used to identify the type of the view in the rendered output. */
public abstract val displayName: String
public val displayName: String

/** The children of this view. */
public abstract val children: Sequence<ScannableView>
public val children: Sequence<ScannableView>

public class AndroidView(public val view: View) : ScannableView() {
override val displayName: String get() = view::class.java.simpleName
public class AndroidView(public val view: View) : ScannableView {
override val displayName: String get() {
val classSimpleName = view::class.java.simpleName
val windowTitle = (view.layoutParams as? WindowManager.LayoutParams)?.title?.toString()
if (windowTitle != null) {
return "$classSimpleName in $windowTitle window-focus:${view.hasWindowFocus()}"
} else {
return classSimpleName
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Why not report focus in this case?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

If there's no window title, then this isn't the root view for an attached window. In which case window focus doesn't make sense. Good question though, should add as comment.

}
}
override val children: Sequence<ScannableView> = view.scannableChildren()

override fun toString(): String = "${AndroidView::class.java.simpleName}($displayName)"
Expand All @@ -45,7 +54,7 @@ public sealed class ScannableView {
public val height: Int,
public val modifiers: List<Modifier>,
override val children: Sequence<ScannableView>
) : ScannableView() {
) : ScannableView {
override fun toString(): String = "${ComposeView::class.java.simpleName}($displayName)"
}

Expand All @@ -57,7 +66,7 @@ public sealed class ScannableView {
* return the error message along with any portion of the tree that was rendered before the
* exception was thrown.
*/
public class ChildRenderingError(private val message: String) : ScannableView() {
public class ChildRenderingError(private val message: String) : ScannableView {
override val displayName: String get() = message
override val children: Sequence<ScannableView> get() = emptySequence()
}
Expand Down
28 changes: 28 additions & 0 deletions radiography/src/test/java/radiography/RadiographyTest.kt
Original file line number Diff line number Diff line change
Expand Up @@ -208,6 +208,34 @@ internal class RadiographyTest {
assertThat(builder.toString()).isEqualTo("${BLANK}View\n")
}

@Test fun `custom ScannableView details reported`() {
class Node(override val displayName: String) : ScannableView {
val mutableChildren = mutableListOf<Node>()

override val children: Sequence<ScannableView>
get() = mutableChildren.asSequence()
}

val root = Node("Root").apply {
mutableChildren += Node("Child A")
mutableChildren += Node("Child B")
}

val prettyHierarchy = Radiography.scan(
scanScope = ScanScope {
listOf(root)
}
)
assertThat(prettyHierarchy).isEqualTo(
"""
|${BLANK}Root
|${BLANK}├─Child A
|${BLANK}╰─Child B
|
""".trimMargin()
)
}

companion object {
private const val BLANK = '\u00a0'
}
Expand Down