Fix typo #4578
Fix typo #4578
Conversation
WalkthroughA single fix in src/js/tabs/setup.js updates the network status rendering to reference downlink (lowercase) instead of downLink, aligning with the actual property used for Mbps display. No public APIs or exports were modified. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~2 minutes Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
|
|
Preview URL: https://f0fb65e3.betaflight-configurator.pages.dev |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/js/tabs/setup.js (2)
63-70: Correct the jQuery selector for default_btnThe selector
"default_btn"targets a non-existent tag rather than the.default_btnclass. Confirmed via searching the HTML templates that elements are marked up as<div class="default_btn">. Update both occurrences insrc/js/tabs/setup.jsto properly disable the button container.• src/js/tabs/setup.js, around line 63
• src/js/tabs/setup.js, around line 67Apply this diff:
- $("default_btn").addClass("disabled"); + $(".default_btn").addClass("disabled"); @@ - $("default_btn").addClass("disabled"); + $(".default_btn").addClass("disabled");
3-3: Rename helper file to match exported symbolThe helper file is named
isExportModeEnabled.jsbut exports a function calledisExpertModeEnabled. Although imports currently reference the file’s misspelled name and work at runtime, the mismatch between “Export” (file) and “Expert” (symbol) is confusing and should be fixed.Please refactor as follows:
- Rename
src/js/utils/isExportModeEnabled.js
→
src/js/utils/isExpertModeEnabled.js- Update all import statements accordingly:
- In
src/js/tabs/setup.js- import { isExpertModeEnabled } from "../utils/isExportModeEnabled"; + import { isExpertModeEnabled } from "../utils/isExpertModeEnabled";- In
src/js/tabs/onboard_logging.js- import { isExpertModeEnabled } from "../utils/isExportModeEnabled"; + import { isExpertModeEnabled } from "../utils/isExpertModeEnabled";- In
src/js/main.js- import { isExpertModeEnabled } from "./utils/isExportModeEnabled.js"; + import { isExpertModeEnabled } from "./utils/isExpertModeEnabled.js";- In
src/js/TuningSliders.js- import { isExpertModeEnabled } from "./utils/isExportModeEnabled"; + import { isExpertModeEnabled } from "./utils/isExpertModeEnabled";This will align the file name with its exported symbol and improve code clarity.
🧹 Nitpick comments (2)
src/js/tabs/setup.js (2)
500-517: Guard against falsy numeric values and improve display formatting for network metrics.Current use of
|| "Unknown"turns legitimate values like0into"Unknown". Also, always appending units yields "Unknown Mbps/ms". Prefer nullish coalescing and format only when numeric. While here, note thateffectiveTypenever equals"none"; that check is redundant.Apply this small, local refactor:
- const type = connection?.effectiveType || "Unknown"; - const downlink = connection?.downlink || "Unknown"; - const rtt = connection?.rtt || "Unknown"; + const type = connection?.effectiveType ?? "Unknown"; + const downlink = connection?.downlink ?? "Unknown"; + const rtt = connection?.rtt ?? "Unknown"; @@ - $(".network-downlink").text(`${downlink} Mbps`); - $(".network-rtt").text(`${rtt} ms`); + $(".network-downlink").text(typeof downlink === "number" ? `${downlink.toFixed(2)} Mbps` : "Unknown"); + $(".network-rtt").text(typeof rtt === "number" ? `${rtt} ms` : "Unknown");Optional (nit): You can drop
|| type === "none"in Line 505 sinceeffectiveTypeis one of"slow-2g" | "2g" | "3g" | "4g". Keeping it doesn’t break anything, it’s just unreachable witheffectiveType.
525-527: Keep network status live by listening to connectivity changes.Right now
showNetworkStatus()runs once on tab init. Hooking to online/offline and Network Information API change events will keep the UI accurate without adding a polling loop.showFirmwareInfo(); showNetworkStatus(); + + // Keep network status updated reactively + window.addEventListener("online", showNetworkStatus); + window.addEventListener("offline", showNetworkStatus); + if (navigator.connection?.addEventListener) { + navigator.connection.addEventListener("change", showNetworkStatus); + }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (1)
src/js/tabs/setup.js(1 hunks)
🔇 Additional comments (1)
src/js/tabs/setup.js (1)
515-515: Correct property fix (Network Information API) — good catch.Using
connection.downlink(lowercase) is the correct property; this prevents "undefined Mbps" renderings and potential runtime surprises.
Summary by CodeRabbit