-
Notifications
You must be signed in to change notification settings - Fork 133
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RUM-4150 [SR] Add Activity Indicator Recorder #1934
RUM-4150 [SR] Add Activity Indicator Recorder #1934
Conversation
…feat/RUM-4150-activity-indicator-recorder
…feat/RUM-4150-activity-indicator-recorder
@@ -46,6 +46,8 @@ internal class SnapshotTestCase: XCTestCase { | |||
function: StaticString = #function | |||
) throws { | |||
show(fixture: fixture) | |||
// Give time for the view to appear and lay out properly | |||
wait(seconds: 0.2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I had to add this, otherwise the snapshot test suite is failing after adding a new test for the activity indicator recorder. This is a temporary workaround for a known issue.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think these tests are relevant anymore, it can be removed IMO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I left the last one which still seems useful to test part of the UnsupportedViewRecorder
logic with specific ViewControllerType
.
shouldRecordImagePredicate: { imageView in | ||
return imageView.image == nil ? false : true | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/nit
shouldRecordImagePredicate: { imageView in | |
return imageView.image == nil ? false : true | |
} | |
shouldRecordImagePredicate: { $0.image != nil } |
/question Are we forcing recording of the image because it is not considered as context image?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, exactly!
…feat/RUM-4150-activity-indicator-recorder
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice!!
What and why?
Adding support for
UIActivityIndicator
in Session Replay.How?
This PR:
Review checklist
Custom CI job configuration (optional)