Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show a toast notification when changing security profiles #1890

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

DanielMcInnes
Copy link
Contributor

For wasm builds only. "Page will automatically reload in 5 seconds" Fixes 1881

@DanielMcInnes
Copy link
Contributor Author

DanielMcInnes commented Feb 5, 2025

Screenshot from 2025-02-05 13-56-01

@DanielMcInnes DanielMcInnes marked this pull request as draft February 5, 2025 04:35
@DanielMcInnes DanielMcInnes force-pushed the 1881-gui-v2-wasm-crashes-when-changing-the-security-profile-from-unsecured-to-weak branch from bb3bf9a to 67f3a33 Compare February 5, 2025 04:40
@DanielMcInnes DanielMcInnes marked this pull request as ready for review February 5, 2025 04:40
@@ -202,6 +202,12 @@ Page {
// This guards the wasm version to trigger a reload even if the reply isn't received.
BackendConnection.securityProtocolChanged()
Global.pageManager.popPage()
if (Qt.platform.os === "wasm") {
Global.showToastNotification(VenusOS.Notification_Info,
//% "Page will automatically reload in 5 seconds"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you change this so it does something like:

Global.showToastNotification(
    VenusOS.Notification_Info,
    BackendConnection.isVrm
        //% "Closing due to security protocol change, you will need to relaunch remote console via VRM"
        ? qsTrId("access_and_security_closing_remote_console")
        //% "Page will automatically reload in 5 seconds"
        : qsTrId("access_and_security_page_will_reload"),
    3000)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now I wonder ... do we actually need to reload the page, for VRM connections?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Honestly, I think we can just add a check like: "if (!isVrm()) return;" at the start of BackendConnection::securityProtocolChanged(), as I don't think we need to reload the page if we are connected to VRM (rather than to the cerbo's web server directly). See for example the similar check in BackendConnection::onNetworkConfigChanged().

Then, I think here in this code, we should just raise the toast notification you originally had, but ONLY if we are NOT connected via VRM. e.g.: add a condition && !BackendConnection.isVrm to the if statement above.

@DanielMcInnes DanielMcInnes force-pushed the 1881-gui-v2-wasm-crashes-when-changing-the-security-profile-from-unsecured-to-weak branch from 67f3a33 to c92c207 Compare February 5, 2025 05:54
For wasm builds only. "Page will automatically reload in 5 seconds"
Part of #1881
@DanielMcInnes DanielMcInnes force-pushed the 1881-gui-v2-wasm-crashes-when-changing-the-security-profile-from-unsecured-to-weak branch from c92c207 to 8efc8e8 Compare February 5, 2025 06:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gui-v2 WASM crashes when changing the security profile from Unsecured to Weak
2 participants