-
Notifications
You must be signed in to change notification settings - Fork 57
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
RUMM-2562 Correct the way we handle orientation change events in SR #1073
RUMM-2562 Correct the way we handle orientation change events in SR #1073
Conversation
Codecov Report
@@ Coverage Diff @@
## feature/session-replay-vo #1073 +/- ##
=============================================================
- Coverage 82.45% 82.41% -0.04%
=============================================================
Files 346 347 +1
Lines 11179 11180 +1
Branches 1849 1853 +4
=============================================================
- Hits 9217 9213 -4
- Misses 1378 1383 +5
Partials 584 584
|
@@ -159,7 +147,7 @@ internal class SnapshotProcessor( | |||
} | |||
} | |||
|
|||
private fun isLastFullSnapshotTime(): Boolean { | |||
private fun resolveIsFullSnapshotTime(): Boolean { |
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: maybe isTimeForFullSnapshort
(but yeah , this may not sound better)?
@@ -226,7 +214,7 @@ internal class SnapshotProcessor( | |||
) | |||
} | |||
|
|||
private fun isNewView( | |||
private fun resolveIsNewView( |
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.
why do we rename this? imo without resolve
it also looks reasonable.
404a00e
to
87fc25d
Compare
What does this PR do?
This PR fixes the way we are handling the orientation change for Session Replay. Previously we were handling this in a separated flow which introduced a new state into the
SnapshotProcessor
causing problems regarding thenew view
detection and the order of the records. With this new approach we are handling theOrientationChanged
event in the same stack used for handling the screen snapshot and by doing this we eliminate the potential issues resulting from introducing a new state for theSnapshotProcessor
.Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
Review checklist (to be filled by reviewers)