redwood icon indicating copy to clipboard operation
redwood copied to clipboard

Use a Redwood schema for loading and error UIs

Open swankjesse opened this issue 1 year ago • 6 comments

Right now loading and error states are pretty hacky! CodeListener has callbacks for each of our 3 states (loading, loaded, detached), and users must write special code for each platform to build views when they receive these callbacks.

I propose we use Redwood’s mechanisms for widgets to implement loading and error states. We would start with a schema like this:

@Schema([DynamicContentRoot::class])
public interface DynamicContent

@Widget(1)
public data class DynamicContentRoot(
  /** 
   * How many different versions of code have been loaded.
   * 
   * This may be used to trigger an animation each time that new code
   * is updated. 
   */
  @Property(1)
  val loadCount: Int,

  /**
   * True if this UI is actively being choreographed by application
   * logic. False if it's not yet loaded, crashes, or otherwise stopped.
   */
  @Property(2)
  val attached: Boolean,

  /** 
   * The current dynamic content. If the application logic is stopped or
   * crashed this will retain a snapshot of the most-recent content, but
   * the content will ignore user actions.
   */
  @Children(1)
  val content: () -> Unit,

  /**
   * The uncaught exception that caused this UI to be detached.
   */
  @Property(3)
  @Default("null")
  val uncaughtException: Throwable? = null,

  /**
   * Call this to restart the UI if is has hung or crashed.
   */
  @Property(4)
  val restart: () -> Unit,
)

For each host platform, the app would implement a binding for DynamicContentRoot.

But the guest wouldn’t know about this schema. Instead, the Treehouse content bindings would synthesize updates in response to app lifecycle events.

swankjesse avatar Jul 09 '24 19:07 swankjesse

I don't think we even need to use the schema mechanism. We can just define a regular interface similar to that which would be generated with a generic type for the view which would be forced by the TreehouseView to match each node type we support. Then each can add or remove as it sees fit.

I have a lot of problems with the existing listener mechanism, so a replacement based on this system sounds excellent.

JakeWharton avatar Jul 09 '24 22:07 JakeWharton

+1 for a standard interface - similar to ChangeListener.

colinrtwhite avatar Jul 10 '24 17:07 colinrtwhite

I took my first pass at this today, and I’ve got some design decisions to make...

Can we expose dynamic content APIs in the Redwood layer?

At the moment RedwoodView has reset() which is the manual version of a code update.

Putting these APIs in the Redwood layer potentially allows us to reduce the overall view hierarchy depth. Given guest code that has content of say Button {} I think we can reasonably expect the host side layout to be one container (a UIStackView or FrameLayout) with the host Button as its only child.

If we put the APIs in the Treehouse layer, it’s awkward to structure the code without introducing a second layer of view hierarchy depth. (2x UIStackView or 2x FrameLayout).

Does RedwoodLayout need to subclass view?

At the moment our iOS class RedwoodUIView is not a UIView subtype, instead it has a UIView property, view.

Our Android class RedwoodLayout is a subclass of FrameLayout which is annoyingly asymmetric with iOS.

I don’t like all of this inheritance, so I’m tempted to rename these classes to avoid implying that (no View suffix), and to introduce some symmetry to them:

public interface RedwoodRootBinding<W : Any> {
  public val contentRoot: ContentRoot<W>
  public val onBackPressedDispatcher: OnBackPressedDispatcher
  public val uiConfiguration: StateFlow<UiConfiguration>
  public val savedStateRegistry: SavedStateRegistry?
}
public interface TreehouseRootBinding<W : Any> : RedwoodRootBinding<W> {
  public val widgetSystem: WidgetSystem<W>
  public val readyForContent: Boolean
  public var readyForContentChangeListener: ReadyForContentChangeListener<W>?
  public var saveCallback: SaveCallback?
  public val stateSnapshotId: StateSnapshot.Id
}

Other challenges

This change potentially introduces new retain cycles, so I’ll have to write tests to defend against that.

This change potentially disrupts state saving, since Android needs IDs in the view hierarchy for that to work. More tests to write!

swankjesse avatar Jul 22 '24 20:07 swankjesse

Can we expose dynamic content APIs in the Redwood layer?

At the moment RedwoodView has reset() which is the manual version of a code update.

Putting these APIs in the Redwood layer potentially allows us to reduce the overall view hierarchy depth. Given guest code that has content of say Button {} I think we can reasonably expect the host side layout to be one container (a UIStackView or FrameLayout) with the host Button as its only child.

Indeed, that is how it behaves today. Whether Compose is running natively or within Zipline the target should be the same RedwoodView.

If we put the APIs in the Treehouse layer, it’s awkward to structure the code without introducing a second layer of view hierarchy depth. (2x UIStackView or 2x FrameLayout).

#1475 tracks the elimination of TreehouseView. Ideally there should only be RedwoodView which supports the composition running natively or within a VM.

Does RedwoodLayout need to subclass view?

At the moment our iOS class RedwoodUIView is not a UIView subtype, instead it has a UIView property, view.

Our Android class RedwoodLayout is a subclass of FrameLayout which is annoyingly asymmetric with iOS.

Subtypes are required on both platforms for their hooks, yes. The reason iOS is not a direct subtype is merely a Kotlin limitation. We cannot subtype an Objective-C type and implement a Kotlin interface at the same time with the source written in Kotlin. A fix would be to rewrite this view entirely in Objective-C.

I don’t like all of this inheritance, so I’m tempted to rename these classes to avoid implying that (no View suffix), and to introduce some symmetry to them:

public interface RedwoodRootBinding<W : Any> {
  public val contentRoot: ContentRoot<W>
  public val onBackPressedDispatcher: OnBackPressedDispatcher
  public val uiConfiguration: StateFlow<UiConfiguration>
  public val savedStateRegistry: SavedStateRegistry?
}
public interface TreehouseRootBinding<W : Any> : RedwoodRootBinding<W> {
  public val widgetSystem: WidgetSystem<W>
  public val readyForContent: Boolean
  public var readyForContentChangeListener: ReadyForContentChangeListener<W>?
  public var saveCallback: SaveCallback?
  public val stateSnapshotId: StateSnapshot.Id
}

This design moves the responsibility of subtyping onto the caller as well as making them responsible for a lot of wiring that has nuance (creating and updating UiConfiguration instances, saving state, etc.).

We already have near-exact versions of these interfaces in RedwoodView+TreehouseView. Our subtypes are there to handle the complexity of wiring everything up since every app will have to do it, and basically do it exactly the same way.

JakeWharton avatar Jul 23 '24 02:07 JakeWharton

I definitely don’t want the user to have to subclass views themselves. Instead our root binding class would return a view that can be added to a layout:

    override func loadView() {
        let emojiSearchLauncher = ...
        let treehouseApp = ...
        let widgetSystem = ...
        let redwoodRootBinding = RedwoodRootBinding(widgetSystem: widgetSystem)
        let content = treehouseApp.createContent(
            source: EmojiSearchContent(),
            codeListener: EmojiSearchCodeListener(treehouseView)
        )
        ExposedKt.bindWhenReady(content: content, view: treehouseRootBinding)
        view = treehouseRootBinding.view
    }

(I screwed up my example above by not exposing the view property directly on the RedwoodRootView binding class. It definitely needs that.)

swankjesse avatar Jul 23 '24 12:07 swankjesse

I guess I'm not entirely opposed to it, but it makes non-fullscreen usage much more difficult as it likely requires using a placeholder FrameLayout and either attempting to replace it at runtime or just adding a layer of nesting. Generally views (on each mobile platform) are user-created.

The widget system doesn't need provided to the view (only the composition or to treehouse), so is the only problem with the current design of library-provided subtypes that iOS' isn't a direct subtype of UIView?

JakeWharton avatar Jul 23 '24 17:07 JakeWharton