Add option to capture replays #579
Conversation
…ing the length in case it ended
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
review notes:
|
| className="switch-root small-switch" | ||
| id="enable-replays" | ||
| onCheckedChange={(checked) => | ||
| project.updateDeviceSettings({ ...deviceSettings, replaysEnabled: checked }) |
There was a problem hiding this comment.
I belive this option should be part of workspace settings (we have a separate interface for that called WorkspaceConfig as it is not a device setting also it would be nice if this option synchronized between computers for users that are logged in to microsoft account
There was a problem hiding this comment.
I thought about it, but feel like it fits the device setting pane better for now.
The workspace settings doesn't have any configuration options except from the IDE panel location for now. Adding it there would make it an isolated option. For me it still feels like its device related because we're recording the device screen in the end. The only thing is that it affects the panel UI as it adds a new button when replays are enabled.
I'd keep it as is for now, and we can consider moving it in the future or once we have more similar options added that'd make it less of a precedence to have a switch button included in the workspace settings dropdown.
There was a problem hiding this comment.
I do not agree, for the following reasons:
WorkspaceConfigcurrently includes showDeviceFrame setting that is affecting user ui in a similar way as replay button does, (it is not included in the settings dropdown but it is accessible through vscode settings panel)- it does not change anything on the device were as all other device settings do, I see your point that it records device but the recording does not happen on the device itself which means that it won't affect how device works during testing. As a user I would see the device setting menu as a menu i use often while testing my application and all other options as dose i set up only when something goes wrong.
- this is a setting that is rarely used (probably only once per user) as a User does not have much reason to use it except their preference which I expect does not change often
- workspace settings persist across devices for logged in user which is a useful functionality that we get for free
- I'm not even sure if this setting needs to exist in the UI because of how rarely it should be used and it's location in the vscode settings would make it easy to remove it from UI settings if they get to cluttered
Thanks for testing. This seemed to be some transient issue that got resolved after I merged main. Will look into other comments |
|
Seems like @p-malecki resolved other minor comments. The only part I don't understand is this comment:
Can you provide more details on what exacly is not working here? |
This PR fixes a bug introduced when adding video replays in #579 that was making the timer freeze and sometimes show negative values during rewind. The issue was caused by updating only the video player's current time and not propagating the change to the element rendering the overlay. We've seemed to rely on `handleTimeUpdate` callback registered on `"timeupdate"` event, but this event is not emitted when updating the current time of the video programmatically. Sometimes, the timer indicated negative values during rewind, and this was also caused by setting the current time on video without updating the overlay's `time` accordingly, which in turn led to `time = 0` (default on mount) and `startTime = [video duration - 5]`, causing the timer to show negative values when `video duration > 5` (we calculate the time displayed on timer as `time - startTime`). The proposed changes include passing a callback that allows the rewind function not only for updating the video's current time, but also the current time of the overlay component. Another change is that we won't show `• REPLAY 0:00` until video metadata is loaded to avoid flickering from `0:00` to `video duration` when starting the rewind. https://github.com/user-attachments/assets/fec826d1-b677-40fb-84cb-a13a792fe0ac https://github.com/user-attachments/assets/b5f07a42-647d-4c34-8fc0-126df618a637 ### How Has This Been Tested: Tested that with introduced changes, the timer behaves as expected both in rewind and normal playback.
This PR adds replays support.
Replays allows the user to capture a recent moment into a video and preview it instantly within the IDE panel.
The current way this feature has been designed is as follows:
Here's what we are adding as part of this PR:
Test plan