feat: add global auto-refresh for monitors - #786
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds app-wide auto-refresh with persisted interval and enablement settings, a navigation popover for refresh controls, a refresh store, and English and Portuguese localization entries. ChangesAuto-refresh controls
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant KenerNav
participant KenerLayout
participant refreshStore
participant AppData
User->>KenerNav: Open auto-refresh controls
KenerNav->>KenerLayout: Render refreshControls snippet
User->>KenerLayout: Set interval or enable refresh
KenerLayout->>refreshStore: Update refresh settings
KenerLayout->>AppData: invalidateAll()
AppData-->>KenerLayout: Refresh completed
KenerLayout->>refreshStore: Record last refresh
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
|---|---|
| src/routes/(kener)/+layout.svelte | Adds auto-refresh logic via onMount with localStorage persistence and timer management; missing translation key for the error toast |
| src/lib/stores/refreshStore.ts | New Svelte 4-style writable store for refresh state; uses legacy pattern instead of Svelte 5 runes recommended by CLAUDE.md |
| src/lib/client/types/refresh.ts | Simple type definition for RefreshState; straightforward and correct |
| src/lib/components/KenerNav.svelte | Accepts an optional controls Snippet prop and renders it before the mobile nav; clean, minimal change |
| src/lib/locales/en.json | Adds six new translation keys for auto-refresh UI; missing the error toast key 'Failed to refresh page data' |
| src/lib/locales/pt-BR.json | Adds Brazilian Portuguese translations for new auto-refresh keys; also missing the error toast key |
Reviews (5): Last reviewed commit: "Merge branch 'main' into feature/auto-re..." | Re-trigger Greptile
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@src/lib/components/KenerNav.svelte`:
- Line 12: Update the `controls` prop declaration in `KenerNav.svelte` to use a
type-only `Snippet` import from `svelte` instead of the inline
`import("svelte").Snippet` type. Preserve the optional prop and existing
`Snippet` typing.
In `@src/lib/stores/refreshStore.ts`:
- Around line 9-12: Define and export a shared default refresh-interval constant
alongside defaultState in refreshStore.ts, then replace the store’s hardcoded
30-second reset value and the layout’s 60-second initialization/fallback values
with that constant so all default refresh behavior stays consistent.
In `@src/routes/`(kener)/+layout.svelte:
- Around line 63-67: The refresh interval handling in +layout.svelte only
enforces a minimum and must also cap values below the browser timer limit. Apply
the same maximum normalization when loading and saving the interval, and
configure the corresponding input with that identical max while preserving the
existing minimum and refreshStore updates.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: eb90f2ae-699b-4301-bffc-e3ea8ed052a7
📒 Files selected for processing (8)
src/lib/components/KenerNav.sveltesrc/lib/components/ui/chart/chart-container.sveltesrc/lib/components/ui/toggle-group/toggle-group.sveltesrc/lib/locales/en.jsonsrc/lib/locales/pt-BR.jsonsrc/lib/server/dns.tssrc/lib/stores/refreshStore.tssrc/routes/(kener)/+layout.svelte
7324c9e to
da56cc3
Compare
|
Caution Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted. Error details |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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 `@src/lib/stores/refreshStore.ts`:
- Around line 3-7: Move the client-only RefreshState type from refreshStore.ts
into the established src/lib/client/types/ location, then import and use that
type in the refresh store. Preserve its existing enabled, interval, and
lastRefresh fields and avoid leaving a duplicate inline definition.
In `@src/routes/`(kener)/+layout.svelte:
- Around line 34-47: The refreshPageData error handler currently logs failures
without notifying the user. Update the catch block to use the existing
Toaster/sonner integration to display a visible refresh-failure notification,
while retaining the console error and existing refreshInProgress cleanup.
🪄 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: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 9264c516-6041-49ae-95ef-67b59853cd43
📒 Files selected for processing (5)
src/lib/components/KenerNav.sveltesrc/lib/locales/en.jsonsrc/lib/locales/pt-BR.jsonsrc/lib/stores/refreshStore.tssrc/routes/(kener)/+layout.svelte
There was a problem hiding this comment.
♻️ Duplicate comments (1)
src/lib/stores/refreshStore.ts (1)
7-14: 📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick winStill unresolved:
RefreshStateshould live insrc/lib/client/types/.This client-only type remains inline despite the prior review flagging it. As per coding guidelines, "Place shared types (used by both client and server) in
src/lib/types/, server-only types insrc/lib/server/types/, and client-only types insrc/lib/client/types/."🤖 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 `@src/lib/stores/refreshStore.ts` around lines 7 - 14, Move the client-only RefreshState type from refreshStore.ts into src/lib/client/types/, then import and reuse it in the refresh store while preserving its existing fields and behavior.Source: Coding guidelines
🤖 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.
Duplicate comments:
In `@src/lib/stores/refreshStore.ts`:
- Around line 7-14: Move the client-only RefreshState type from refreshStore.ts
into src/lib/client/types/, then import and reuse it in the refresh store while
preserving its existing fields and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1f451f64-b71f-47a5-b089-bf8dde09e3b5
📒 Files selected for processing (3)
src/lib/components/KenerNav.sveltesrc/lib/stores/refreshStore.tssrc/routes/(kener)/+layout.svelte
Summary
This PR adds a global auto-refresh option to the public status page.
Users can enable or disable auto-refresh, configure the refresh interval, and the selected settings are saved locally so they persist across page reloads.
Changes
invalidateAll()Testing
npm run checknpm run buildRelated to #533.
Summary by CodeRabbit