RUMM-2964 [SR] Make view-tree recording more customizable

[ncreated]

What and why?

📦 This PR adds more flexibility to the way SR traverses and records subtree hierarchies.

This change will allow each NodeRecorder (e.g. UIViewRecorder or UILabelRecorder) defining their own strategies and will be soon leveraged in UIPickerViewRecorder (and later in UIDatePickerViewRecorder).

How?

Before this change, only two strategies existed, denoted by recordSubtree: Bool flag. For a given hierarchy, either all subviews were recorded or all could be ignored:

In result, the recorder was visiting following nodes in DFS order:

ROOT, A, AA, AB, ABA, ABB, B, C

With this PR, we're adding one more strategy, so changing recordSubtree boolean into an enum:

internal enum NodeSubtreeStrategy {
    case record // equivalent of `recordSubtree: true`
    case ignore // equivalent of `recordSubtree: false`
    case replace(subtreeNodes: [Node]) // new one ⭐️
}

By using the new .replace(subtreeNodes:) strategy, certain node recorders (e.g. UIPickerViewRecorder) will be able to opt-out from their subtree traversal and instead replace the subtree with its curated [Node] information:

ROOT, A, AA, AB, ABA, ABB, B, C, CV1, CV2, ..., CVN

This will enable code reuse for certain view-tree recorders, meaning:

• the root view-tree recorder could be configured with all node recorders (UIViewRecorder, UILabelRecorder, UISwitchRecorder, UISliderRecorder, ...) - as we can expect any node in the root view's hierarchy;
• the UIViewPickerRecorder could opt-in only UILabelRecorder and UIViewRecorder - as we expect only labels and shapes in picker's subtree;
• similar, date-picker recorder would expect texts + shapes, whereas text-field recorder could additionally search for images (to record icons).

Last not least, this required switching from tree-like nodes structure:

struct ViewTreeSnapshot {
   // ...

   let root: Node
}

struct Node {
   // ...

   let childNodes: [Node]
}

to flat array given by DFS-visit order:

struct ViewTreeSnapshot {
   // ...

   /// Nodes in DFS order.
   let nodes: [Node]
}

struct Node {
   // ...
}

The change of data structure should have no impact on the SR performance - nodes were already visited in DFS order, here we just add them to an array while visiting (instead of allocating a graph).

Review checklist

• Feature or bugfix MUST have appropriate tests (unit, integration)
• Make sure each commit and the PR mention the Issue number or JIRA reference
• Add CHANGELOG entry for user facing changes

Custom CI job configuration (optional)

• Run unit tests
• Run integration tests
• Run smoke tests

[maxep]

Great idea, and testing is on point!

What I don't understand tho is how you will create the .replace strategy when recording 🤔 The UIPickerViewRecorder will need to depend on a UILabelRecorder for creating the nodes?

[ncreated]

Great idea, and testing is on point!

What I don't understand tho is how you will create the .replace strategy when recording 🤔 The UIPickerViewRecorder will need to depend on a UILabelRecorder for creating the nodes?

Not directly, but yes. The UIPickerViewRecorder will depend on ViewTreeRecorder. The way we separate "any subtree tree recording" responsibility (now done solely by ViewTreeRecorder) in this PR enables this kind of composition and reuse.

In pseudocode, the root view (app window) could use:

let appWindowSubtreeRecorder = ViewTreeRecorder(
   nodeRecorders: [
      UIViewRecorder(),
      UILabelRecorder(),
      UISwitchRecorder(),
      UIToggleRecorder(),
      UITabBarRecorder(),
      UIImageViewRecorder(),
      // ...
      UIPickerViewRecorder()
   ]
)

And any concrete recorder can specify its own list of expected nodes, e.g.:

let pickerSubtreeRecorder = ViewTreeRecorder(
   nodeRecorders: [
      UIViewRecorder(),
      UILabelRecorder(),      
   ]
)

This way, the pickerSubtreeRecorder.recordNodes(for: picker, in: context) ran in UIPickerViewRecorder will give it only nodes that describe labels or shapes - so the picker's implementation can further reason about these elements in its own context and curate them for SR approximated look (e.g. try to infer the "text for selected option" and "filter out all shapes that we don't want in SR").

The appWindowSubtreeRecorder.recordNodes(for: appWindow, in: context) in turn will return nodes for all elements in the window. Because it could enter and hit UIPickerViewRecorder at some point, with this PR we enable "picker recorder" to decide by its own on how to record its own subtree.

Main idea here is to achieve composable architecture, so compelx node recorders could reuse simple ones in their own subtrees. An alternative to this would be making each node recorder heavily parameterized, for example adding an option in UIImageViewRecorder so it will opt-out from executing if it is ran in UIPickerView's subtree. In our earlier discussion with @maciejburda we rejected that idea as it seems more complex and less maintainable (doesn't promote OCP for sure).

Hope it makes it cleaner now @maxep - LTM!

[maxep]

Yes, I see! Then I'm concerned about having 2 logics for recording, one 'kind' of recorder won't depend on other recorders and will just delegate the tree traversing to its caller, they will return the root node semantics only. The other kind will depend on its own set of recorders and will provide the root node semantics and its children nodes.

If we generalise, we could say that a recorder is responsible for traversing the view tree, it could use all default recorders (equivalent to strategy .record), a subset of recorders (~ strategy .replace), or none (~ strategy .ignore). It would all come down to the recorder, and this would be enforced by a simple design:

internal protocol NodeRecorder {

    /// - Returns: The full list of nodes representing the view and subviews.
    func nodes(of view: UIView, with attributes: ViewAttributes, in context: ViewTreeRecordingContext) -> [Node]
}

Recorders would use helper functions to traverse the subviews recursively, and they will decide what nodes to return, they will use the provided attributes and semantics they found.

It's a bigger refactor I admit, so I left it as a suggestion but I believe it will scale better as we add more and more recorders. LMWYT!

[ncreated]

internal protocol NodeRecorder {

    /// - Returns: The full list of nodes representing the view and subviews.
    func nodes(of view: UIView, with attributes: ViewAttributes, in context: ViewTreeRecordingContext) -> [Node]
}

As we discussed on the meeting - it is an interesting proposal and a bit aligned with current design 👍. We may want to look into it later - for now we think it is best to cover more use cases with current architecture before considering a shift. We're still learning the complexity of views hierarchy and discovering more arch drivers.