RecorderView: FieldView Replay #162
Conversation
A feature to review past runs on FieldView. Data is stored locally in browser, or can be downloaded and uploaded.
| replayOverlay?: { | ||
| ops: DrawOp[]; | ||
| }; |
There was a problem hiding this comment.
I think replayOverlay should be separate from TelemetryItem. Live telemetry received from the server comes in batches which complicates the handling logic. Field recording playback should include one overlay at a time and that overlay should live outside of the telemetry item array.
| this.overlay = this.props.telemetry.reduce((acc, { field, replayOverlay, fieldOverlay }) => ({ | ||
| ops: [ | ||
| ...acc.ops, | ||
| ...(field?.ops || []), | ||
| ...(replayOverlay?.ops || []), | ||
| ...(fieldOverlay?.ops || []), | ||
| ], | ||
| }), { ops: [] }); |
There was a problem hiding this comment.
I don't think live overlays and replayed overlays should be displayed simultaneously. I think it probably makes sense to favor replays over live. In that case, the old logic for setting this.overlay should be preserved with a separate switch between this.overlay and the replay overlay from this.props.
| <div className="canvas-container" style={{ marginBottom: '.5em' }}> | ||
| <AutoFitCanvas ref={this.canvasRef} containerHeight="calc(100% - 3em)" /> | ||
| </div> |
| ); | ||
| this.setState({ savedReplays: keys }, () => { | ||
| if (this.state.autoSelect) { | ||
| this.handleLoadTelemetryByFilename({ target: { selectedOptions: Array.from(this.state.savedReplays.map(filename => ({ value: filename }))) } }); |
There was a problem hiding this comment.
I prefer new code to be written in TS. No need to update this whole file at this point though.
| const maxStorageSize = 5 * 1024 * 1024; | ||
| if (totalSize + newDataSize > maxStorageSize) { | ||
| alert("Cannot save replay: LocalStorage quota exceeded."); | ||
| return; | ||
| } else { | ||
| localStorage.setItem(storageKey, dataToSave); | ||
| } |
There was a problem hiding this comment.
It would be nice to set a per-replay limit and truncate recordings that exceed it. This should probably be done with a limit on the size of this.telemetryRecording; I commented elsewhere about that.
| this.telemetryRecording.push({ | ||
| timestamp: relativeTimestamp, | ||
| ops: overlay.ops, | ||
| }); |
There was a problem hiding this comment.
The length of this array should be limited. It should probably also be implemented with a circular buffer to avoid allocating / shifting items around.
| }; | ||
|
|
||
| handleLoadTelemetryByFilename = (event) => { | ||
| const selectedFiles = Array.from(event.target.selectedOptions, (option) => option.value); |
There was a problem hiding this comment.
Why Array.from() here instead of map()?
| this.telemetryReplay = []; | ||
|
|
||
| selectedFiles.forEach((filename) => { | ||
| const savedTelemetry = localStorage.getItem(filename); | ||
| if (savedTelemetry) { | ||
| const parsedTelemetry = JSON.parse(savedTelemetry); | ||
| this.telemetryReplay.push(parsedTelemetry); | ||
| } | ||
| }); |
There was a problem hiding this comment.
Should this go in the setState() callback?
|
|
||
| handleLoadTelemetryByFilename = (event) => { | ||
| const selectedFiles = Array.from(event.target.selectedOptions, (option) => option.value); | ||
| if (selectedFiles.length === 0) return; |
|
Very cool. Do you happen to have a video of this in operation? |
A feature to review past runs on FieldView. Data is stored locally in browser, or can be downloaded and uploaded.