fix: replay storybook scene rot #23383
Conversation
pauldambra
changed the title
|
Size Change: +224 B (+0.02%) Total Size: 1.06 MB ℹ️ View Unchanged
|
| @@ -621,7 +621,7 @@ export const sessionRecordingPlayerLogic = kea<sessionRecordingPlayerLogicType>( | |||
| // Check for the "t" search param in the url on first load | |||
| if (!cache.hasInitialized) { | |||
| cache.hasInitialized = true | |||
| const searchParams = fromParamsGivenUrl(window.location.search) | |||
| const searchParams = router.values.searchParams | |||
There was a problem hiding this comment.
window.location.search was picking up the storybook iframe searchparams not the applications apparent search params
| @@ -736,6 +736,7 @@ export const sessionRecordingsPlaylistLogic = kea<sessionRecordingsPlaylistLogic | |||
| } | |||
| ] => { | |||
| const params: Params = objectClean({ | |||
| ...router.values.searchParams, | |||
There was a problem hiding this comment.
this was replacing all search params which makes it hard to do things like add t when you know it should work
There was a problem hiding this comment.
these playlist views are, I guess, broken only in these snapshots 🙈
| // KLUDGE: when loaded for visual regression tests we want to pause the player | ||
| // but only after it has had time to buffer and show the frame | ||
| actions.setPause() | ||
| }, 100) |
There was a problem hiding this comment.
a little yucky but it should let us have a test that proves we can render a recording without flapping 🤞
There was a problem hiding this comment.
Could we use waitForSelector in the test instead?
There was a problem hiding this comment.
Frustratingly if we start paused we never process the data so the player frame is just a black square. If we play (the default behaviour and then stop after its processed the data then we see the player screen and can assert that at least the full snapshot has been processed (i.e. we didn't completely break rrweb playback)
We have to be paused so that the visual regression snapshot doesn't flap because of the seekbar timestamp changing
And don't want to be at 0 so we can see that the sekbar at least paints the "played" portion of the recording correctly
There was a problem hiding this comment.
I've added more to the comment so that the future traveller who wants to avoid this slightly yucky reaching into implementation code from tests knows what I was thinking
| // KLUDGE: when loaded for visual regression tests we want to pause the player | ||
| // but only after it has had time to buffer and show the frame | ||
| actions.setPause() | ||
| }, 100) |
There was a problem hiding this comment.
Could we use waitForSelector in the test instead?
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions <41898282+github-actions[bot]@users.noreply.github.com>
we don't snapshot the replay scenes and so they're falling out of sync with reality, a few fixes to remove some error toasts and console error noise when viewing them