feat(webapp): integrate the RDP and VNC WASM packages #1329

RRRadicalEdward · 2025-05-13T07:35:00Z

This PR changes the Devolutions Gateway's webapp to integrate the new modular VNC and RDP packages.
Key changes:

Refactored the webapp to use the updated API from iron-remote-desktop
Update the VNC connection form with new specific settings: Enabled Encodings, Disable Cursor, etc.
Update the ARD connection form with Resolutions Quality and Quality Mode settings.
Enhanced the viewer toolbar; added Toggle Cursor Style button and Unicode Keyboard Mode checkbox.

Also, I've tested integration, and all 3 protocols are working as expected.
Here are some demos:

VNC.mp4

RDP.mp4

ARD.mp4

webapp/src/client/app/modules/web-client/ssh/web-client-ssh.component.ts

webapp/src/client/app/modules/web-client/ard/web-client-ard.component.ts

webapp/tsconfig.json

RRRadicalEdward · 2025-05-13T07:50:37Z

@CBenoit Please review when you get a chance

CBenoit

This the PR is quite big, would you be okay with adding "Copilot" as a reviewer and let it perform a first pass? I think I’m not able to add it myself, could you try on your side?
https://docs.github.com/en/copilot/using-github-copilot/code-review/using-copilot-code-review

webapp/package-lock.json

RRRadicalEdward · 2025-05-13T08:54:49Z

This the PR is quite big, would you be okay with adding "Copilot" as a reviewer and let it perform a first pass?

Yes. Sure

I think I’m not able to add it myself, could you try on your side? https://docs.github.com/en/copilot/using-github-copilot/code-review/using-copilot-code-review

I'm afraid I can't do it either. I can't set a reviewer because of a lack of rights

CBenoit · 2025-05-13T09:06:22Z

This PR changes the Devolutions Gateway's webapp to integrate the new modular VNC and RDP packages. Key changes:

* Refactored the webapp to use the updated API from `iron-remote-desktop`

* Update the _VNC_ connection form with new specific settings: `Enabled Encodings`, `Disable Cursor`, etc.

* Update the _ARD_ connection form with `Resolutions Quality` and `Quality Mode` settings.

* Enhanced the viewer toolbar; added `Toggle Cursor Style` button and `Unicode Keyboard Mode` checkbox.

Great job!

VNC.mp4

It’s now possible to configure all the codecs, excellent!

A few questions and suggestions:

What about enabling all of them by default? I think most people don’t really care about what is enabled until they want to disable some specific codecs for various reasons, so we should let the negotiation happens with all the codecs by default.
What does "Disable cursor" refers to?
I would prefer we state the flags positively: "Enable cursor" (checked by default) instead of "Disable cursor" (unchecked by default). It’s easier to scan. (I think this is generally true and does not apply only to UI.)
Do we support "non-Unicode input" at all for VNC? I’m pretty sure Unicode input should be hardcoded to "true" for VNC and ARD.

RDP.mp4

IIRC, we do not support Unicode input for RDP yet? This is planned, but until it’s actually implemented, could you hide the checkbox (comment it out with a comment explaining this should be enabled back later).

(Also, this should not be checked by default for RDP in my opinion.)

ARD.mp4

As mentioned above, Unicode input should maybe be hardcoded to "true", with no checkbox.

CBenoit · 2025-05-13T09:09:55Z

I think I’m not able to add it myself, could you try on your side? https://docs.github.com/en/copilot/using-github-copilot/code-review/using-copilot-code-review

I'm afraid I can't do it either. I can't set a reviewer because of a lack of rights

Maybe because you are not part of the organization. Since I can’t assign it on my side either, it’s likely not possible to request a review at all in this case.

package-lock.json

webapp/src/client/app/shared/resolvers/wasm-init.resolver.ts

...client/form/form-controls/resolution-quality-control/resolution-quality-control.component.ts

webapp/src/client/app/modules/web-client/rdp/web-client-rdp.component.ts

webapp/src/client/app/modules/web-client/telnet/web-client-telnet.component.ts

CBenoit · 2025-05-13T10:44:44Z

webapp/src/client/app/modules/web-client/form/form-components/ssh/ssh-form.component.ts

-      this.formService.addControlToForm(
-        this.form,
-        'authMode',
+      this.formService.addControlToForm({
+        formGroup: this.form,
+        controlName: 'authMode',
        inputFormData,
-        true,
-        false,
-        SshAuthMode.Username_and_Password,
-      );
+        isRequired: true,
+        defaultValue: SshAuthMode.Username_and_Password,
+      });


praise: I think it’s more readable that way, especially for the booleans 👍 (named parameters)

CBenoit

I’ve gone through the PR! Many improvements, good job! To be picky: I think it’s better to open smaller PRs for the small refactorings unrelated to the main change, and it’s easier to review all the things separately.

RRRadicalEdward · 2025-05-13T11:13:07Z

What does "Disable cursor" refers to?

It disables the Cursor pseudo-encoding which tells the server to draw the mouse cursor into the framebuffer.
The user may want to disable it to improve performance, because it slightly reduces the server's load.

I would prefer we state the flags positively: "Enable cursor" (checked by default) instead of "Disable cursor" (unchecked by default). It’s easier to scan. (I think this is generally true and does not apply only to UI.)

Done.

RRRadicalEdward · 2025-05-13T11:14:25Z

Do we support "non-Unicode input" at all for VNC? I’m pretty sure Unicode input should be hardcoded to "true" for VNC and ARD.

Yeah. I removed it for VNC and ARD

RRRadicalEdward · 2025-05-13T11:17:50Z

IIRC, we do not support Unicode input for RDP yet? This is planned, but until it’s actually implemented, could you hide the checkbox (comment it out with a comment explaining this should be enabled back later).

(Also, this should not be checked by default for RDP in my opinion.)

I commented it out

RRRadicalEdward · 2025-05-13T11:18:33Z

ARD.mp4

As mentioned above, Unicode input should maybe be hardcoded to "true", with no checkbox.

Done

RRRadicalEdward · 2025-05-13T11:19:05Z

What about enabling all of them by default? I think most people don’t really care about what is enabled until they want to disable some specific codecs for various reasons, so we should let the negotiation happens with all the codecs by default.

Done

CBenoit · 2025-05-13T12:54:07Z

IIRC, we do not support Unicode input for RDP yet? This is planned, but until it’s actually implemented, could you hide the checkbox (comment it out with a comment explaining this should be enabled back later).

(Also, this should not be checked by default for RDP in my opinion.)

I commented it out

I'm sorry, you were right: we support it already. My only suggestion is to uncheck it by default (this is the default behavior in the official client too I believe).

webapp/package.json

CBenoit · 2025-05-13T15:08:11Z

What does "Disable cursor" refers to?

It disables the Cursor pseudo-encoding which tells the server to draw the mouse cursor into the framebuffer. The user may want to disable it to improve performance, because it slightly reduces the server's load.

Got you! I think that if the server supports it, we may receive cursor images that we handle ourselves, right? And I understand that in this case the performance is not badly impacted.

What about modifying this way?

Checkbox label: Enable cursor pseudo-encoding (a bit, jargony, but specific 🤔)
Add a tooltip: When enabled, the server may render the mouse cursor into the framebuffer, which can affect performance depending on the server capabilities

RRRadicalEdward · 2025-05-13T15:45:18Z

IIRC, we do not support Unicode input for RDP yet? This is planned, but until it’s actually implemented, could you hide the checkbox (comment it out with a comment explaining this should be enabled back later).

(Also, this should not be checked by default for RDP in my opinion.)

I commented it out

I'm sorry, you were right: we support it already. My only suggestion is to uncheck it by default (this is the default behavior in the official client too I believe).

Done

CBenoit

LGTM! Excellent work!

CBenoit · 2025-05-13T23:31:24Z

Can you address this comment #1329 (comment) in a follow up PR? Thanks!

RRRadicalEdward · 2025-05-14T07:14:19Z

Can you address this comment #1329 (comment) in a follow up PR? Thanks!

Done in #1335

feat(webapp): integrate the RDP and VNC WASM packages

122290c

RRRadicalEdward commented May 13, 2025

View reviewed changes

webapp/src/client/app/modules/web-client/ssh/web-client-ssh.component.ts Outdated Show resolved Hide resolved

RRRadicalEdward commented May 13, 2025

View reviewed changes

webapp/src/client/app/modules/web-client/ard/web-client-ard.component.ts Show resolved Hide resolved

RRRadicalEdward commented May 13, 2025

View reviewed changes

webapp/tsconfig.json Outdated Show resolved Hide resolved