Skip to content

🤖 Fix: Show a popup to alert users when they are offline or have a poor network connection (Resolves #120)#121

Open
github-actions[bot] wants to merge 1 commit into
mainfrom
opencode/issue-120-27503055208
Open

🤖 Fix: Show a popup to alert users when they are offline or have a poor network connection (Resolves #120)#121
github-actions[bot] wants to merge 1 commit into
mainfrom
opencode/issue-120-27503055208

Conversation

@github-actions

Copy link
Copy Markdown
Contributor

AI: Resolves #120

This Pull Request was automatically generated by OpenCode to address Issue #120.

📝 AI Modification Summary & Conclusion:

Implementation Summary: Offline / Poor Network Alert Popup

Objective

Show a popup banner to alert users when they are offline or have a poor network connection, inform them that the application is reading data from cache, and support i18n for all messages.

Files Modified

1. src/i18n/en.ts, src/i18n/zh.ts, src/i18n/de.ts, src/i18n/fr.ts, src/i18n/ja.ts

  • Added a new network translation section in each language file with three keys:
    • offline — "You are currently offline" (translated per locale)
    • poorConnection — "Your network connection is unstable" (translated per locale)
    • usingCache — "Reading data from cache" (translated per locale)

2. src/services/useNetworkStatus.ts (new file)

  • Created a Vue 3 Composition API composable that monitors:
    • navigator.onLine + browser online/offline events for binary connectivity
    • Network Information API (navigator.connection) for connection quality (effectiveType and saveData)
  • Returns two reactive refs:
    • isOnlinetrue/false reflecting browser online status
    • isPoorConnectiontrue when effectiveType is 'slow-2g' or '2g', or saveData is enabled
  • Properly registers and cleans up event listeners via onMounted/onUnmounted

3. src/components/utils/NetworkStatusBanner.vue (new file)

  • A fixed-position banner component displayed at the top of the viewport
  • Uses Vue <Transition> for smooth slide-in/out animation
  • Two visual states:
    • --offline (red background, white text) when the user is completely disconnected
    • --poor (yellow background, dark text) when the connection is slow/unstable
  • Shows an icon, a status message (network.offline or network.poorConnection), and a secondary cache notice (network.usingCache)
  • Uses useI18n() for translated messages
  • Accessible with role="alert" and aria-live="assertive"

4. src/App.vue

  • Imported and rendered <NetworkStatusBanner> before <CookieNotice> at the top of the app
  • Imported useNetworkStatus composable and passed its reactive state (isOnline, isPoorConnection) as props to the banner

Technical Decisions

  • Composables pattern — Followed the existing Vue 3 Composition API style used throughout the project
  • No external dependencies — Relies only on browser-native APIs (navigator.onLine, Network Information API) and Vue built-ins
  • i18n-first — All user-facing strings are defined in the network translation section and accessed via useI18n() t() function — consistent with the existing i18n architecture
  • Banner vs Notification — Chose a top-of-page fixed banner (over Naive UI corner notifications) for maximum visibility and persistence
  • Graceful degradation — If the Network Information API is unavailable, the composable gracefully falls back to only tracking online/offline events
  • Component pattern — Follows the existing CookieNotice.vue component structure (reactive visibility, scoped CSS, flex layout) for consistency

@github-actions

Copy link
Copy Markdown
Contributor Author

🤖 Codex-Style Code Review by OpenCode

💡 This is an automated advisory review. It is non-blocking and does not affect merge requirements.

PR Review: Network Status Alert Banner

Summary

A well-executed feature adding an offline/poor-network banner to the app. The composable pattern is consistent with the existing codebase, i18n is properly integrated, and the component follows CookieNotice.vue conventions. The implementation is clean and lightweight with no external dependencies.

PR Size: Medium (~160 lines)
Review Depth: Standard


🎉 Strengths

  • Composable separationuseNetworkStatus.ts cleanly encapsulates browser API logic, lifecycle management, and cleanup into a reusable unit
  • Graceful degradation — The getConnection() fallback chain for vendor prefixes and the onMounted guard ensure the feature degrades gracefully
  • SSR awarenesstypeof navigator !== 'undefined' check in isOnline initialization correctly prevents SSR crashes
  • i18n consistency — All user-facing strings follow the existing i18n architecture with translations in all 5 supported locales
  • Accessibilityrole="alert" on the banner is correct for live region announcements
  • Follows existing patternsNetworkStatusBanner.vue mirrors the structure, BEM naming, and scoped CSS approach of CookieNotice.vue

🔴 Blocking Issues

None identified.


🟡 Important Issues

1. Prop type mismatch: isOnline declared as boolean | null but never null

src/components/utils/NetworkStatusBanner.vue:25:

defineProps<{
  isOnline: boolean | null
  isPoorConnection: boolean
}>()

src/services/useNetworkStatus.ts:53:

const isOnline = ref(typeof navigator !== 'undefined' ? navigator.onLine : true)  // Ref<boolean>

The composable always produces a boolean, but the prop allows null. This creates a misleading contract — a future caller could pass null, which would cause the banner to silently hide (since null === false is false and null && ... is falsy), which might not be the desired behavior for an "unknown connectivity" state.

Suggestion: Either:

  • Remove null from the union and make it isOnline: boolean
  • Or if an "unknown" loading state is intentionally supported, handle it with a separate visual or keep the banner hidden until known

2. Edge case: navigator.onLine may not exist on some platforms

src/services/useNetworkStatus.ts:18:

const isOnline = ref(typeof navigator !== 'undefined' ? navigator.onLine : true)

If navigator exists but navigator.onLine is not a property (very old browsers, some web views), navigator.onLine evaluates to undefined, producing ref(undefined) which is Ref<boolean> in type but undefined at runtime. The template checks isOnline === falseundefined === false is false, so the banner stays hidden, which is arguably the right default, but the type system doesn't catch this.

Suggestion:

const isOnline = ref(typeof navigator !== 'undefined' && navigator.onLine !== undefined ? navigator.onLine : true)

3. white-space: nowrap may cause horizontal overflow on mobile

src/components/utils/NetworkStatusBanner.vue:80:

.network-banner__text {
  font-weight: 600;
  white-space: nowrap;
}

On narrow viewports (e.g., 320px width), long translations like "Your network connection is unstable" with nowrap could overflow the viewport. The parent has flex-wrap: wrap and justify-content: center, so the cache notice will wrap, but the text span itself won't break. If the text alone exceeds the viewport, it will overflow.

Suggestion: Either remove white-space: nowrap or add overflow-wrap: break-word as a safety net.


🟢 Minor Nits

1. Redundant aria-live with role="alert"

src/components/utils/NetworkStatusBanner.vue:12:

role="alert"
aria-live="assertive"

The alert role implicitly maps to aria-live="assertive" per the ARIA spec. The explicit aria-live is redundant. Consider removing it to avoid confusion and keep the template clean.


2. Vue <Transition> won't animate on initial mount if already visible

If the page loads while offline, the banner mounts as visible — the enter transition only plays when the element is inserted into the DOM via v-if toggling. This is standard Vue behavior and not technically a bug, but it means the first appearance lacks the slide-in animation that users would see on subsequent online→offline transitions.

Not actionable unless this is considered undesirable UX.


3. No test coverage

No tests are included for useNetworkStatus.ts. The composable has nontrivial logic (event listener setup/teardown, browser API detection, computed quality assessment). Consider adding unit tests to verify:

  • Event listeners are registered in onMounted and removed in onUnmounted
  • updateConnectionQuality correctly classifies slow-2g/2g as poor
  • saveData flag triggers poor connection state

🔒 Security Considerations

  • No hardcoded secrets
  • No external data input or injection vectors
  • No DOM manipulation beyond conditional rendering

Verdict

✅ Approve — This is a clean, well-structured feature that follows the project's conventions. The three 🟡 items above are suggestions worth addressing but not blocking.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Show a popup to alert users when they are offline or have a poor network connection

0 participants