Skip to content

Conversation

@haslinghuis
Copy link
Member

@haslinghuis haslinghuis commented Sep 15, 2025

  • after chrome version 140 multiple select in onboard logging no longer works.
image

Summary by CodeRabbit

  • New Features
    • Revamped Onboard Logging “Debug Fields” UI: replaced multi-select with per-field checkboxes and real-time enable/disable (changes apply immediately, no save step).
  • Refactor
    • Simplified debug-fields population and removed legacy multi-select behavior.
  • Style
    • Added a table-based layout inside a styled box and moved the section title into the header for clarity.
  • Localization
    • Updated English label: “Data included” → “Debug Fields included”.

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

coderabbitai bot commented Sep 15, 2025

Walkthrough

Switches the Blackbox debug-fields selector from a multi-select to a checkbox table UI, updating the bitmask in real time via checkbox changes instead of on Save. Updates the English locale label for the section. Corresponding HTML is restructured to host a table for dynamic rows.

Changes

Cohort / File(s) Summary
Locale label update
locales/en/messages.json
Updated value of onboardLoggingDebugFields from "Data included" to "Debug Fields included".
UI logic refactor to checkbox table
src/js/tabs/onboard_logging.js
Removed multi-select logic and Save-time mask aggregation; added createDebugTableRow(i, enabled) to build per-field checkbox rows; populateDebugFields now renders rows; checkboxes update FC.BLACKBOX.blackboxDisabledMask on change; removed sortSelect().multipleSelect() usage.
HTML structure for new UI
src/tabs/onboard_logging.html
Replaced select element with a GUI box containing a table body .blackboxDebugFieldsTable for dynamic rows; moved label into spacer_box_title.

Sequence Diagram(s)

sequenceDiagram
  participant U as User
  participant UI as Debug Fields Table (checkboxes)
  participant BB as FC.BLACKBOX
  participant Save as Save Action

  rect rgb(245,245,255)
    note right of UI: New flow: real-time updates
    U->>UI: Toggle checkbox for field i
    UI->>BB: Update blackboxDisabledMask bit i (set/clear)
    UI-->>U: Checkbox reflects current state
  end

  rect rgb(245,255,245)
    note right of Save: Save persists current mask
    U->>Save: Click "Save Settings"
    Save->>BB: Read current blackboxDisabledMask
    Save-->>U: Save complete
  end
Loading
sequenceDiagram
  participant U as User
  participant MS as Multi-select (old)
  participant BB as FC.BLACKBOX
  participant Save as Save Action

  note right of MS: Old flow (removed)
  U->>MS: Select/deselect fields
  U->>Save: Click "Save Settings"
  Save->>MS: Read selected options
  Save->>BB: Compute and assign blackboxDisabledMask
  Save-->>U: Save complete
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • nerdCopter
  • Quick-Flash
  • blckmn
✨ 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 05bcac8 and 4574e92.

📒 Files selected for processing (3)
  • locales/en/messages.json (1 hunks)
  • src/js/tabs/onboard_logging.js (2 hunks)
  • src/tabs/onboard_logging.html (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
  • locales/en/messages.json
  • src/tabs/onboard_logging.html
  • src/js/tabs/onboard_logging.js

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.

Pre-merge checks

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The PR description only states that Chrome v140 broke the multiple-select and includes a screenshot, but it does not follow the repository's description template and is missing important reviewer information such as reproduction steps, expected vs actual behavior, a brief summary of what files/changes address the issue, test or CI results, and any linked issue numbers, so reviewers cannot fully assess risk or completeness. Please expand the PR description to follow the repository template: include a concise summary of the change, clear steps to reproduce and the expected versus actual behavior, a short note of which files or approach were changed, how it was tested and CI status (or test instructions), and any related issue numbers; remove any leftover template text and confirm the target branch.
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 "Chrome v140 workaround - Replace multiple select in onboard logging" clearly and succinctly states the primary change: replacing the multi-select used in onboard logging as a workaround for Chrome v140; it directly matches the changes in the PR (multi-select → per-field checkboxes) and is specific enough for a teammate scanning history.

@haslinghuis haslinghuis moved this to Bugfix in 2025.12.0 Sep 15, 2025
@haslinghuis haslinghuis force-pushed the replace-multipleselect branch from 05bcac8 to 2d87ab9 Compare September 15, 2025 20:55
@haslinghuis haslinghuis force-pushed the replace-multipleselect branch from 2d87ab9 to 4574e92 Compare September 15, 2025 21:00
@sonarqubecloud
Copy link

@github-actions
Copy link
Contributor

@haslinghuis haslinghuis moved this from Bugfix to App in 2025.12.0 Sep 15, 2025
@haslinghuis haslinghuis changed the title Replace multiple select in onboard logging Chrome v140 workaround - Replace multiple select in onboard logging Sep 15, 2025
Copy link
Member

@nerdCopter nerdCopter left a comment

Choose a reason for hiding this comment

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

nice

@haslinghuis haslinghuis merged commit 8a17799 into betaflight:master Sep 16, 2025
8 checks passed
@github-project-automation github-project-automation bot moved this from App to Done in 2025.12.0 Sep 16, 2025
@haslinghuis haslinghuis deleted the replace-multipleselect branch September 16, 2025 20:39
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.

4 participants