square / radiography

Text-ray goggles for your Android UI.
Apache License 2.0
851 stars 35 forks source link

Compose 1.4 breaks AndroidView parsing #153

Closed elihart closed 11 months ago

elihart commented 1 year ago

We upgraded to compose 1.4 and our radiography graphs now no longer pick up AndroidView interop layers within our Composable UIs. As far as I can tell this is because Radiography expects a Ref holding a reference to the android view, and the AndroidView composable was rewritten a bit to not do this.

Instead, LayoutNode within compose now has a interopViewFactoryHolder that seems to play this role instead.

Any plans to update Radiography to support this?

pyricau commented 1 year ago

We should support it yes. I don't have the bandwidth, someone should do that work :) .

elihart commented 11 months ago

FYI I'm taking a stab at this now. I will try to update to kotlin 1.9.10 and compose 1.5.3 and then fix this issue, as well as some other things that broke with the compose APIs. Will require updating some other versions as well

elihart commented 11 months ago

looks like it's an easy fix, we just have to access the view like this

private fun Group.androidViewChildren(): List<AndroidViewInfo> {
  return data.mapNotNull { datum ->
    (datum as? InteroperableComposeUiNode)
      ?.getInteropView()
      ?.let(::AndroidViewInfo)
  }
}

I'll put up a PR next week.

My main question is whether we want this to be backwards compatible. i'm not sure that it can be since it seems like InteroperableComposeUiNode is a new interface.

The compose-unsupported-tests module is also giving me trouble after bumping to kotlin 1.9.10 since it is trying to use an old version of compose that isn't compatible with the updated kotlin version.

elihart commented 11 months ago

I've submitted a fix for this at https://github.com/square/radiography/pull/157