Skip to content

fix(LogViewer): restore hover-driven chart cursor after extraction#14407

Open
AhmWael wants to merge 1 commit into
mavlink:masterfrom
AhmWael:master
Open

fix(LogViewer): restore hover-driven chart cursor after extraction#14407
AhmWael wants to merge 1 commit into
mavlink:masterfrom
AhmWael:master

Conversation

@AhmWael
Copy link
Copy Markdown
Contributor

@AhmWael AhmWael commented May 18, 2026

Description

When LogViewerChart was extracted from LogViewerPage in #14354, the chart MouseArea lost hoverEnabled and the logic that updates the cursor while the pointer moves. The vertical marker and value popup only updated on a short click-release, which regressed inspection of DataFlash (.bin) and ULog charts.

This PR restores the pre-extraction desktop behavior:

  • LogViewerChart: hoverEnabled on desktop, cursor updates on press and while moving (when not drag-zooming), marker hidden when the pointer leaves the chart, marker X refreshed when plotArea layout changes
  • LogViewerAltChart: same hover/press/move/exit behavior for the map altitude chart and map marker sync

Mobile remains click-only (hoverEnabled disabled via ScreenTools.isMobile).

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation update
  • Refactoring (no functional changes)
  • CI/Build changes
  • Other

Testing

  • Tested locally
  • Added/updated unit tests
  • Tested with simulator (SITL)
  • Tested with hardware

Manual verification:

  1. Open Analyze → Log Viewer and load a .bin (or .ulg) log.
  2. On the Charting tab, select one or more fields.
  3. Move the mouse over the chart — the vertical marker and popup should follow the pointer and show time, mode, and field values (Current / Min / Max).
  4. Click-drag to zoom; right-click to reset zoom.
  5. On the Map tab (if GPS/altitude data exists), confirm the altitude chart cursor behaves the same and the map position dot tracks the cursor.

Platforms Tested

  • Linux
  • Windows
  • macOS
  • Android
  • iOS

Flight Stacks Tested

  • PX4
  • ArduPilot

Checklist

  • I have read the Contribution Guidelines
  • I have read the Code of Conduct
  • My code follows the project's coding standards
  • I have added tests that prove my fix/feature works
  • New and existing unit tests pass locally

By submitting this pull request, I confirm that my contribution is made under the terms of the project's dual license (Apache 2.0 and GPL v3).

When LogViewerChart was extracted from LogViewerPage (mavlink#14354), the chart
MouseArea lost hoverEnabled and the handler that updates the cursor while
the pointer moves. The marker and value popup only updated on a short
click-release, which regressed .bin chart inspection.

Restore the pre-extraction interaction on desktop:
- LogViewerChart: hoverEnabled, cursor update on press and on move (when not drag-zooming), hide marker on exit, refresh marker X when plotArea changes
- LogViewerAltChart: same hover/press/move/exit behavior for the map altitude chart

Mobile keeps click-only interaction (hoverEnabled disabled via ScreenTools).
Copilot AI review requested due to automatic review settings May 18, 2026 19:45
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Note

Copilot was unable to run its full agentic suite in this review.

Adds hover-driven cursor updates and improves cursor/marker behavior in LogViewer charts (primarily on non-mobile), including hiding the marker when the pointer exits and keeping marker position in sync with plot-area changes.

Changes:

  • Enable hover tracking on desktop (hoverEnabled: !ScreenTools.isMobile) to update cursor position without clicking.
  • Update cursor position on press/move/release flows and hide marker on pointer exit.
  • Refresh cursor pixel position when the chart plot area changes (main chart).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
src/AnalyzeView/LogViewer/LogViewerChart.qml Adds hover support, updates cursor handling around zoom interactions, hides marker on exit, and refreshes marker position on plot-area changes.
src/AnalyzeView/LogViewer/LogViewerAltChart.qml Adds hover support, updates cursor handling around zoom interactions, and clears marker on exit.

Comment on lines 602 to +611
const dragWidth = _zoomSelectionRect.width
_zoomSelectionRect.visible = false
if (dragWidth < ScreenTools.defaultFontPixelWidth * 0.5) {
_updateCursorInfo(mouse.x, mouse.y, width, height)
return
}
const leftX = _pixelToAxisX(_zoomSelectionRect.x)
const rightX = _pixelToAxisX(_zoomSelectionRect.x + _zoomSelectionRect.width)
applyZoomRange(Math.min(leftX, rightX), Math.max(leftX, rightX))
_updateCursorInfo(mouse.x, mouse.y, width, height)
}
Comment on lines 297 to +306
const dragW = _zoomRect.width
_zoomRect.visible = false
if (dragW < ScreenTools.defaultFontPixelWidth * 0.5) {
root._updateCursor(mouse.x)
return
}
const leftX = root._pixelToAxisX(_zoomRect.x)
const rightX = root._pixelToAxisX(_zoomRect.x + _zoomRect.width)
root._applyZoom(Math.min(leftX, rightX), Math.max(leftX, rightX))
root._updateCursor(mouse.x)
}
@codecov
Copy link
Copy Markdown

codecov Bot commented May 18, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 25.57%. Comparing base (f29efd3) to head (4e43bfa).
⚠️ Report is 18 commits behind head on master.

❌ Your project check has failed because the head coverage (25.57%) is below the target coverage (30.00%). You can increase the head coverage or adjust the target coverage.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #14407      +/-   ##
==========================================
+ Coverage   25.47%   25.57%   +0.10%     
==========================================
  Files         769      769              
  Lines       65912    66244     +332     
  Branches    30495    30640     +145     
==========================================
+ Hits        16788    16940     +152     
+ Misses      37285    37281       -4     
- Partials    11839    12023     +184     
Flag Coverage Δ
unittests 25.57% <ø> (+0.10%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.
see 23 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update c5c1488...4e43bfa. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@github-actions
Copy link
Copy Markdown
Contributor

Build Results

Platform Status

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

Some builds failed.

Pre-commit

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

Pre-commit hooks: 4 passed, 45 failed, 7 skipped.

Test Results

linux-coverage: 90 passed, 0 skipped
Total: 90 passed, 0 skipped

Code Coverage

Coverage: 59.5%

No baseline available for comparison

Artifact Sizes

Artifact Size
QGroundControl 216.87 MB
QGroundControl-aarch64 176.80 MB
QGroundControl-linux 335.18 MB
QGroundControl-mac 187.14 MB
QGroundControl-windows 187.17 MB
QGroundControl-x86_64 171.96 MB
No baseline available for comparison

Updated: 2026-05-18 20:37:41 UTC • Triggered by: Android

@AhmWael
Copy link
Copy Markdown
Contributor Author

AhmWael commented May 18, 2026

@DonLakeFlyer Can you take a look at this PR?

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.

2 participants