Skip to content

fix(dialog): label vehicle reboot prompt Reboot/Continue - #14976

Open
dakejahl wants to merge 1 commit into
masterfrom
dakejahl/reboot-dialog-buttons
Open

fix(dialog): label vehicle reboot prompt Reboot/Continue#14976
dakejahl wants to merge 1 commit into
masterfrom
dakejahl/reboot-dialog-buttons

Conversation

@dakejahl

Copy link
Copy Markdown
Collaborator

Summary

Rename the vehicle-reboot-required dialog buttons from Ok/Cancel to Reboot/Continue.

Problem

Ok is the highlighted action and reboots the vehicle. Configuring several reboot-required parameters makes it easy to reboot after each change.

Solution

Keep the Ok/Cancel roles, override the labels to Reboot and Continue, and match the prompt text. Continue dismisses; Reboot still reboots.

Ok was the highlighted action and rebooted the vehicle, so changing several reboot-required parameters (PWM protocol per timer, etc.) caused accidental reboots.
@github-actions

Copy link
Copy Markdown
Contributor

Build Results

Platform Status

Platform Status Details
Linux Passed View
Windows Passed View
MacOS Passed View
Android Passed View

All builds passed.

Pre-commit

Check Status Details
pre-commit Failed (non-blocking) View

Pre-commit hooks: 2 passed, 75 failed, 7 skipped.

Test Results

linux-coverage-integration: 50 passed, 0 skipped
linux-coverage-unit: 184 passed, 0 skipped
linux-sanitizers-integration: 29 passed, 0 skipped
linux-sanitizers-unit: 184 passed, 0 skipped
Total: 447 passed, 0 skipped

Code Coverage

Coverage: 70.4%

No baseline available for comparison

Artifact Sizes

Artifact Size
QGroundControl 253.07 MB
QGroundControl-aarch64 176.80 MB
QGroundControl-installer-AMD64 138.50 MB
QGroundControl-installer-AMD64-ARM64 80.76 MB
QGroundControl-installer-ARM64 109.32 MB
QGroundControl-linux 84.74 MB
QGroundControl-mac 84.74 MB
QGroundControl-x86_64 189.04 MB
No baseline available for comparison

Updated: 2026-08-27 18:41:54 UTC • Commit: 169e4c3 • Triggered by: Windows

title: dialogTitle,
text: dialogText + " " + qsTr("Click Reboot to reboot the vehicle now."),
buttons: Dialog.Ok | Dialog.Cancel,
acceptButtonText: qsTr("Reboot"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Huh! I never thought of just doing that. I think it works...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Oh, wait you modded the dialog code as well. I see.

@DonLakeFlyer DonLakeFlyer added this to the Release V5.2 milestone Aug 29, 2026

@fallenmi fallenmi left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Reviewed exact head 169e4c3204284421eedc94dcc6d83f17978b9798.

The custom labels preserve the existing dialog semantics: the reboot prompt still uses Dialog.Ok | Dialog.Cancel, Reboot remains the accepted action, and Continue remains the rejected/dismiss action. The accepted path still null-checks the active vehicle before calling rebootVehicle(), while the Cancel role keeps Escape dismissal enabled.

Verification:

  • A focused exact-source QML/JavaScript oracle passed on the submitted head and a conflict-free integration tree against live master: custom labels, accept/reject routing, close behavior, and both active-vehicle/null-vehicle callback paths were exercised for 20 iterations across 10 repeated runs.
  • git diff --check passed.
  • Exact-head CI reports 47 successful checks (plus one skipped documentation deploy), including 447 passing tests, Linux ASan+UBSan, platform builds, CodeQL, and grype.

No blocking issues found.

Disclosure: I used OpenAI Codex to assist with inspecting the exact diff, repository instructions and interaction history, checking CI, and constructing the focused oracle. I verified the evidence and own this review.

@dakejahl dakejahl left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

What's the verdict @DonLakeFlyer ? Shall we merge it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants