From 58fcf859ba7018bb40e564597d978e1e8093a98d Mon Sep 17 00:00:00 2001 From: filip131311 <159789821+filip131311@users.noreply.github.com> Date: Fri, 20 Dec 2024 12:43:22 +0100 Subject: [PATCH] Improve device settings (#869) This PR improves the behavior of the device settings, by: 1) We now invoke `updatedDeviceSettings` only when the user releases the "font size" slider, this change prevents issues with applications not being able to handle updates in quick succession and better models how the end user would change the font size. 2) we only send `replaysEnabled` and `showTouches` to the sim-server when the new setting is a change compared to the previous one preventing sim-server from logging errors about setting already being enabled like the following: ``` sim-server: video_error replay already exists ``` ### How Has This Been Tested: run test application and see if the `updatedDeviceSettings` is invoked only once per slide and if sim-server is affected by changes unrelated to it --- .../src/project/deviceSession.ts | 22 +++++++++++-------- .../components/DeviceSettingsDropdown.tsx | 2 +- 2 files changed, 14 insertions(+), 10 deletions(-) diff --git a/packages/vscode-extension/src/project/deviceSession.ts b/packages/vscode-extension/src/project/deviceSession.ts index f99730510..de66b43be 100644 --- a/packages/vscode-extension/src/project/deviceSession.ts +++ b/packages/vscode-extension/src/project/deviceSession.ts @@ -316,17 +316,21 @@ export class DeviceSession implements Disposable { } public async changeDeviceSettings(settings: DeviceSettings): Promise { - this.deviceSettings = settings; - if (settings.replaysEnabled && !this.isLaunching) { - this.device.enableReplay(); - } else { - this.device.disableReplays(); + if (this.deviceSettings?.replaysEnabled !== settings.replaysEnabled && !this.isLaunching) { + if (settings.replaysEnabled) { + this.device.enableReplay(); + } else { + this.device.disableReplays(); + } } - if (settings.showTouches && !this.isLaunching) { - this.device.showTouches(); - } else { - this.device.hideTouches(); + if (this.deviceSettings?.showTouches !== settings.showTouches && !this.isLaunching) { + if (settings.showTouches) { + this.device.showTouches(); + } else { + this.device.hideTouches(); + } } + this.deviceSettings = settings; return this.device.changeSettings(settings); } diff --git a/packages/vscode-extension/src/webview/components/DeviceSettingsDropdown.tsx b/packages/vscode-extension/src/webview/components/DeviceSettingsDropdown.tsx index 718af3a37..721fec67a 100644 --- a/packages/vscode-extension/src/webview/components/DeviceSettingsDropdown.tsx +++ b/packages/vscode-extension/src/webview/components/DeviceSettingsDropdown.tsx @@ -103,7 +103,7 @@ function DeviceSettingsDropdown({ children, disabled }: DeviceSettingsDropdownPr defaultValue={[contentSizes.indexOf(deviceSettings.contentSize)]} max={6} step={1} - onValueChange={([value]) => { + onValueCommit={([value]) => { project.updateDeviceSettings({ ...deviceSettings, contentSize: contentSizes[value],