Skip to content

Error view is never added as a child view controller #668

Description

@jkmassel

displayError calls didMove(toParent:) on a UIHostingController it never added as a child, so UIKit view-controller containment is never actually established.

Detail

ios/Sources/GutenbergKit/Sources/EditorViewController.swift:1040-1042:

self.errorViewController = UIHostingController(rootView: AnyView(view))
self.displayAndCenterView(errorViewController!.view)
self.errorViewController?.didMove(toParent: self)

addChild(_:) is what establishes the parent-child relationship; didMove(toParent:) is only the notification you send after it. Without the addChild, the hosting controller has no parent, so it is outside the containment hierarchy and receives no appearance or trait-collection callbacks.

The teardown half is missing too: hideError() (:1046) only does errorViewController?.view.removeFromSuperview() — no willMove(toParent: nil), no removeFromParent(), and it does not nil out errorViewController. It also has zero callers anywhere in ios/, which is its own problem (see #667).

How it bites

Anything the error view needs to react to stops working: Dynamic Type changes, light/dark mode switches, rotation, and size-class changes are never propagated to the hosting controller. Cosmetic today, because the content is a static ContentUnavailableView — but it is a trap for anyone who makes that view interactive or adds a retry button to it, which is exactly what #667 proposes.

There is also a force-unwrap on the line between (errorViewController!.view) that a plain let would avoid.

Suggested fix

let controller = UIHostingController(rootView: AnyView(view))
addChild(controller)
displayAndCenterView(controller.view)
controller.didMove(toParent: self)
self.errorViewController = controller

and the mirror in hideError():

errorViewController?.willMove(toParent: nil)
errorViewController?.view.removeFromSuperview()
errorViewController?.removeFromParent()
errorViewController = nil

Found while reviewing #651. Pre-existing; that PR does not touch this path.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    [Type] BugAn existing feature does not function as intended[Type] Code QualityIssues or PRs that relate to code qualityiOS

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions