feat: show recording button if set #26565
Conversation
|
Size Change: 0 B Total Size: 1.11 MB ℹ️ View Unchanged
|
📸 UI snapshots have been updated2 snapshot changes in total. 0 added, 2 modified, 0 deleted:
Triggered by this commit. |
| ? properties.$recording_status | ||
| ? properties.$recording_status === 'active' | ||
| : true | ||
| : false |
There was a problem hiding this comment.
| : false | |
| : isSemVerAfter("1.194.0") ? false : true |
so.... we need to consider library version here, no?
something like that - with a made up method I don't think exists 🙈
There was a problem hiding this comment.
Do we not get away without it? I was working off the assumption that properties.$recording_status will be undefined for older versions of the SDK. That will fail the second check in the function and hence resolve to true (eg. for all events in a time before we captured $recording_status we assume there might be a recording
There was a problem hiding this comment.
ugh... yeah, ok... too many question marks and colons 🫠
return properties.$session_id
? properties.$recording_status
? properties.$recording_status === 'active'
: true
: false
```
| @@ -38,7 +36,13 @@ export const OverviewTab = (): JSX.Element => { | |||
| event ? ( | |||
| <div className="h-full overflow-auto"> | |||
| <div className="bg-bg-light p-1 flex justify-end border-b min-h-[42px]"> | |||
| <ViewSessionButton event={event} /> | |||
| {mightHaveRecording(event.properties) && ( | |||
| <ViewRecordingButton | |||
There was a problem hiding this comment.
maybe nicer to do this mightHaveRecording check inside the component and set disabledReason on the button
There was a problem hiding this comment.
I didn't want to pass the properties into the component because not ever instance has access to them. Have changed it around so disabledReason is passed in as a prop in the two cases we know the event properties
Problem
Follow up to PostHog/posthog-js#1571
Changes
maybeHasRecordingfunction that looks at the$recording_statusproperty