fix(network): footer Local toggle was dead in Tauri (window.confirm no-op) - #169
Conversation
The footer Local/Network pill called window.confirm() before enabling, but window.confirm is a no-op in the Tauri webview (returns false), so the enable action was silently swallowed — the button appeared to do nothing. Replace it with a reliable in-app confirm popover (Cancel/Enable). The backend endpoint was always working (verified: enable opens a real listener on the share port). Adds a regression test for the Local -> confirm -> Enable -> POST flow. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughNetworkToggle component replaces browser ChangesIn-app confirmation for network sharing enable
🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add 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. Comment |
|
| Filename | Overview |
|---|---|
| frontend/src/components/NetworkToggle.jsx | Replaces window.confirm() with an in-app confirm popover using a new confirming state; logic is correct, error/busy handling is sound, no regressions introduced. |
| frontend/src/components/NetworkToggle.css | Adds button styles for the new confirm popover actions; clean and self-contained, consistent with existing design tokens. |
| frontend/src/components/NetworkToggle.test.jsx | Adds regression test covering Local → confirm popover → Enable → POST flow; beforeEach/afterEach correctly saves and restores global.fetch, fetch mock is complete and correctly asserts on the POST endpoint. |
Sequence Diagram
sequenceDiagram
participant User
participant Pill as Local Pill Button
participant Panel as Confirm Popover
participant API as Backend API
User->>Pill: click (network disabled)
Pill->>Panel: setConfirming(true)
Panel-->>User: Show "Share on your network?" panel
alt User clicks Cancel
User->>Panel: click Cancel
Panel->>Pill: setConfirming(false)
else User clicks Enable
User->>Panel: click Enable
Panel->>API: POST /system/network/enable
API-->>Panel: "{ enabled: true, pin, share_port, lan_addresses }"
Panel->>Pill: setSt(...), setConfirming(false), setOpen(true)
Pill-->>User: Pill shows "Network" (on state)
Panel-->>User: Show network details panel (QR, PIN, addresses)
else API call fails
API-->>Panel: error
Panel-->>User: toast.error("Could not enable sharing")
Note over Panel: confirming stays true — user can retry or cancel
end
Reviews (1): Last reviewed commit: "fix(network): footer Local toggle dead i..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
| {!st.enabled && confirming && ( | ||
| <div className="net-toggle__panel net-toggle__panel--confirm"> | ||
| <div className="net-toggle__panel-title">Share on your network?</div> | ||
| <p className="net-toggle__hint"> | ||
| Other devices on your Wi-Fi/Ethernet will be able to reach OmniVoice | ||
| using the access PIN shown once it's on. | ||
| </p> | ||
| <div className="net-toggle__confirm-actions"> | ||
| <button type="button" className="net-toggle__cancel" onClick={() => setConfirming(false)} disabled={busy}>Cancel</button> | ||
| <button type="button" className="net-toggle__enable" onClick={enable} disabled={busy}>{busy ? 'Enabling…' : 'Enable'}</button> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
When the confirm panel mounts, focus stays on the pill button. Keyboard users must tab through to reach Cancel/Enable, and there is no Escape handler to close the panel. A screen reader also has no role="dialog" or aria-labelledby to announce the prompt. While the pill-click toggle and Cancel button do dismiss it, adding autoFocus to the Cancel button and a keydown listener for Escape would bring this in line with accessibility expectations for a confirmation flow.
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/components/NetworkToggle.jsx`:
- Around line 64-76: The confirmation panel in NetworkToggle.jsx currently
contains hardcoded strings; replace them with i18n keys by calling the
translation function (t) for the title, description, and button labels used in
the JSX: replace "Share on your network?" and the long description with
t('networkToggle.confirm.title') and t('networkToggle.confirm.description'), and
replace "Cancel", "Enable", and "Enabling…" with
t('networkToggle.confirm.cancel'), t('networkToggle.confirm.enable') and
t('networkToggle.confirm.enabling') respectively; ensure the component
imports/receives the t function (or use the existing i18n hook) and add the
corresponding keys to the locale files so the confirming panel (rendered when
!st.enabled && confirming) uses translated strings while preserving the existing
handlers setConfirming, enable, and the busy state.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: a48bb277-a2ab-4640-ac2d-ce3fb6f2cbe0
📒 Files selected for processing (3)
frontend/src/components/NetworkToggle.cssfrontend/src/components/NetworkToggle.jsxfrontend/src/components/NetworkToggle.test.jsx
| {!st.enabled && confirming && ( | ||
| <div className="net-toggle__panel net-toggle__panel--confirm"> | ||
| <div className="net-toggle__panel-title">Share on your network?</div> | ||
| <p className="net-toggle__hint"> | ||
| Other devices on your Wi-Fi/Ethernet will be able to reach OmniVoice | ||
| using the access PIN shown once it's on. | ||
| </p> | ||
| <div className="net-toggle__confirm-actions"> | ||
| <button type="button" className="net-toggle__cancel" onClick={() => setConfirming(false)} disabled={busy}>Cancel</button> | ||
| <button type="button" className="net-toggle__enable" onClick={enable} disabled={busy}>{busy ? 'Enabling…' : 'Enable'}</button> | ||
| </div> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Hardcoded UI text violates i18n guideline.
The confirmation panel contains hardcoded English strings ("Share on your network?", "Other devices on your Wi-Fi/Ethernet will be able to reach OmniVoice using the access PIN shown once it's on.", "Cancel", "Enable", "Enabling…") that must go through the i18n translation layer using t('...') keys. As per coding guidelines, all user-facing text in frontend/src/**/*.{vue,ts,tsx,js,jsx} must use the i18n translation layer, never hardcode text.
🌐 Example refactor to use i18n
First, add translation keys to your locale files (e.g., locales/en.json):
{
"networkToggle.confirm.title": "Share on your network?",
"networkToggle.confirm.description": "Other devices on your Wi-Fi/Ethernet will be able to reach OmniVoice using the access PIN shown once it's on.",
"networkToggle.confirm.cancel": "Cancel",
"networkToggle.confirm.enable": "Enable",
"networkToggle.confirm.enabling": "Enabling…"
}Then update the component to use t():
+import { useTranslation } from 'react-i18next'; // or your i18n library
export default function NetworkToggle() {
+ const { t } = useTranslation();
const [st, setSt] = useState({ enabled: false });
// ...
{!st.enabled && confirming && (
<div className="net-toggle__panel net-toggle__panel--confirm">
- <div className="net-toggle__panel-title">Share on your network?</div>
+ <div className="net-toggle__panel-title">{t('networkToggle.confirm.title')}</div>
<p className="net-toggle__hint">
- Other devices on your Wi-Fi/Ethernet will be able to reach OmniVoice
- using the access PIN shown once it's on.
+ {t('networkToggle.confirm.description')}
</p>
<div className="net-toggle__confirm-actions">
- <button type="button" className="net-toggle__cancel" onClick={() => setConfirming(false)} disabled={busy}>Cancel</button>
+ <button type="button" className="net-toggle__cancel" onClick={() => setConfirming(false)} disabled={busy}>{t('networkToggle.confirm.cancel')}</button>
- <button type="button" className="net-toggle__enable" onClick={enable} disabled={busy}>{busy ? 'Enabling…' : 'Enable'}</button>
+ <button type="button" className="net-toggle__enable" onClick={enable} disabled={busy}>{busy ? t('networkToggle.confirm.enabling') : t('networkToggle.confirm.enable')}</button>
</div>
</div>
)}As per coding guidelines: All user-facing text in the UI must go through the i18n translation layer using t('...') keys in locales/*.json files, never hardcode non-English text.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@frontend/src/components/NetworkToggle.jsx` around lines 64 - 76, The
confirmation panel in NetworkToggle.jsx currently contains hardcoded strings;
replace them with i18n keys by calling the translation function (t) for the
title, description, and button labels used in the JSX: replace "Share on your
network?" and the long description with t('networkToggle.confirm.title') and
t('networkToggle.confirm.description'), and replace "Cancel", "Enable", and
"Enabling…" with t('networkToggle.confirm.cancel'),
t('networkToggle.confirm.enable') and t('networkToggle.confirm.enabling')
respectively; ensure the component imports/receives the t function (or use the
existing i18n hook) and add the corresponding keys to the locale files so the
confirming panel (rendered when !st.enabled && confirming) uses translated
strings while preserving the existing handlers setConfirming, enable, and the
busy state.
1 participant
The footer Local pill did nothing because
enable()gated onwindow.confirm(...), which is a no-op in the Tauri webview (returns false) — so it bailed before calling the (working) enable endpoint. Replaced with a reliable in-app confirm popover (Cancel / Enable).The backend was never the problem — verified on a live instance that
POST /system/network/enableopens a real TCP listener on the configured share port (5050) with real LAN/Tailscale addresses + PIN. This was purely the frontend confirm. Adds a regression test for Local → confirm → Enable → POST.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes