REPLAY-1339 Support UITextField elements in SR
#1191
Conversation
| / \ | ||
| I1 V1 | ||
| */ | ||
| func testFlattenNodes_withInvisibleNode() { |
There was a problem hiding this comment.
Invisible nodes are no longer passed to NodesFlattener - so this test doesn't make sense anymore 💡
| / | ||
| V1 | ||
| */ | ||
| func testFlattenNodes_withMixedVisibleAndInvisibleNodes() { |
There was a problem hiding this comment.
Invisible nodes are no longer passed to NodesFlattener - so this test doesn't make sense anymore 💡
| XCTAssertEqual(builder.buildWireframes(with: WireframesBuilder()).count, 2) | ||
| } | ||
|
|
||
| func testWhenImageViewHasImageOrAppearance() throws { |
There was a problem hiding this comment.
This test was duplicating configuration and assertions from the tests defined above. Getting rid of it 🧽.
| @@ -34,7 +34,8 @@ internal struct UISegmentRecorder: NodeRecorder { | |||
| } | |||
| }() | |||
| ) | |||
| return SpecificElement(wireframesBuilder: builder, subtreeStrategy: .ignore) | |||
| let node = Node(viewAttributes: attributes, wireframesBuilder: builder) | |||
| return SpecificElement(subtreeStrategy: .ignore, nodes: [node]) | |||
There was a problem hiding this comment.
feels like we could have convenience function that would allow us to initialise the SpecificElement with attributes, builder and subtree strategy 🤔
There was a problem hiding this comment.
THB I don't think this is good idea. Except saving few lines of code (in entire PR) this won't achieve any architectural goal, but only make things more coupled. SpecificElement would have to additionally depend on ViewAttributes and NodeWireframesBuilder - even if we call it convenience function, that would count as dependency and make concept less readable: Why passing strategy, attributes and builder to some SpecificElements vs strategy and nodes to others?
What and why?
📦 This PR enhances the recording of
UITextFieldelements in session replay. Its preliminary support was introduced earlier, but here we solve remaining glitches, improve PIIs masking and adjust colors for light and dark mode.On PIIs masking - following the SR Privacy Restrictions now we replace passwords, e-mails and phone numbers with
"xxx".How?
The work done to
UITextFieldRecorderleverages concepts introduced earlier in #1175.Unfortunately, the
subtreeStrategy: .replace(subtreeNodes:)turned out to be limiting for text fields situation. Although we now use sub-recorders inUITextFieldRecorder:the actual "text" information isn't available anywhere in TF's subtree. Instead, it is accessible through the root node (
textField.text) and we can't use the strategy of replacing subtree nodes to replace the root one.For that reason, I had to do one more iteration on
.subtreeStrategyconcept. What turns out to be sustainable (I made a PoC proving it to work for remaining Input Elements) is changing 1:1 relationship between aNodeandNodeSemantics:- node.semantics = <single semantics>into 1:N relationship between
NodeSemanticsandNode:+ semantics.nodes = <an array of nodes>This works fine for text fields, as now the
UITextFieldRecordercan return 2 nodes:It doesn't change anything to other recorders, although I had to update their unit tests.
Review checklist
Custom CI job configuration (optional)