Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Sep 25, 2025

  • fixes some issues found after 1960bd9

Summary by CodeRabbit

  • New Features

    • Improved GPS tab stability by cleaning up the map when closing, reducing memory use and preventing slowdowns.
  • Bug Fixes

    • Increased reliability of GPS settings (protocol, baud rate, SBAS) so selections apply consistently.
  • Style

    • Updated SBAS control container and label styling for a clearer, more consistent GPS settings UI.
  • Refactor

    • Simplified GPS control bindings to match the updated UI structure for easier maintenance.

@haslinghuis haslinghuis added this to the 2025.12 milestone Sep 25, 2025
@haslinghuis haslinghuis self-assigned this Sep 25, 2025
@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 25, 2025

Walkthrough

Updated GPS tab DOM selectors in JS to match HTML class changes and added a gps.cleanup(callback) method to destroy the map instance and free resources.

Changes

Cohort / File(s) Change Summary
GPS tab JS selectors & cleanup
src/js/tabs/gps.js
Replaced selectors: .number.gps_protocol.gps_protocol, .number.gps_ubx_sbas.gps_ubx_sbas (deriving group via closest(".select")), .number.gps_baudrate.gps_baudrate. Added gps.cleanup(callback) to destroy the map instance and invoke an optional callback.
GPS tab HTML structure
src/tabs/gps.html
Changed UBX SBAS control wrapper from div.number.gps_ubx_sbas to div.select; label span changed to span.freelabel while keeping same i18n key and select/options unchanged.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User
  participant UI as GPS Tab (HTML)
  participant JS as gps.js
  participant Map as Map Instance

  User->>UI: open GPS tab
  UI->>JS: init()
  JS->>Map: create map instance
  Note right of Map: Map initialized

  User->>UI: navigate away / close tab
  UI->>JS: cleanup()
  JS->>Map: destroy()
  JS-->>UI: optional callback()
  Note over JS,Map: resources released
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested labels

RN: UI

Suggested reviewers

  • blckmn
  • nerdCopter
  • VitroidFPV

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning There is no pull request description provided, so the template sections required for context, background, and testing instructions are entirely missing and the generic boilerplate was not replaced with meaningful content. Please populate the PR description by removing the template instructions and adding sections such as a summary of changes, rationale, and testing steps, following the repository’s description template to aid reviewers.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title Check ✅ Passed The title “Fix GPS tab” directly reflects the main purpose of the changeset by indicating that the GPS tab functionality is being repaired, and it is concise and specific enough for team members to understand at a glance.
✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9b197be and 90e397e.

📒 Files selected for processing (2)
  • src/js/tabs/gps.js (1 hunks)
  • src/tabs/gps.html (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/tabs/gps.html
🧰 Additional context used
🧬 Code graph analysis (1)
src/js/tabs/gps.js (1)
src/js/tabs/ports.js (1)
  • gpsBaudrateElement (164-164)
🔇 Additional comments (2)
src/js/tabs/gps.js (2)

106-117: Selectors now align with the new markup

Switching to the plain .gps_* selectors and using .closest(".select") ensures the handlers and toggles bind to the updated DOM structure so the entire control hides/shows correctly.


458-465: Map instance cleanup looks good

Destroying the stored map instance and nulling the reference prevents the OpenLayers objects from lingering after the tab unloads. Thanks for wiring the optional callback as well.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@haslinghuis haslinghuis moved this to App in 2025.12.0 Sep 25, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c5e676f and 9b197be.

📒 Files selected for processing (2)
  • src/js/tabs/gps.js (1 hunks)
  • src/tabs/gps.html (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/js/tabs/gps.js (1)
src/js/tabs/ports.js (1)
  • gpsBaudrateElement (164-164)
🔇 Additional comments (1)
src/tabs/gps.html (1)

64-69: No CSS selectors reference .gps_ubx_sbas. Search across CSS/LESS yielded zero matches.

@haslinghuis haslinghuis requested a review from Copilot September 25, 2025 22:50
Copy link
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

This PR fixes styling and selector issues in the GPS tab to improve reliability and consistency. The changes address UI presentation issues with GPS settings controls and update JavaScript selectors to match the revised HTML structure.

  • Updated HTML structure for the GPS SBAS control container and label styling
  • Modified JavaScript selectors to align with the new HTML structure
  • Improved consistency in GPS settings UI presentation

Reviewed Changes

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

File Description
src/tabs/gps.html Updated SBAS control container class and label styling for better consistency
src/js/tabs/gps.js Updated JavaScript selectors to match revised HTML structure

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

Preview URL: https://pr4633.betaflight-app.pages.dev

@haslinghuis haslinghuis merged commit ac661b6 into betaflight:master Sep 26, 2025
8 checks passed
@haslinghuis haslinghuis deleted the fix-gps-tab branch September 26, 2025 16:44
@github-project-automation github-project-automation bot moved this from App to Done in 2025.12.0 Sep 26, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

3 participants