feat(dashboard): config diff enhancements — unsaved indicator, Ctrl+S, undo, per-section revert by BillChirico · Pull Request #258 · VolvoxLLC/volvox-bot

BillChirico · 2026-03-07T20:23:03Z

Summary

Improvements to the config diff/save flow.

Changes

Unsaved indicator: Save button tooltip now shows count + names of changed sections
Ctrl+S / Cmd+S: Keyboard shortcut triggers save; no-op when focused in input/textarea/select
Undo last save: 30-second auto-dismiss timer on the undo button after each save
Per-section revert: Inline diff view now shows section badges with revert buttons (RotateCcw)

Closes #133

- Yellow dot appears on Save button when draft differs from saved config - Tooltip shows the count and names of changed sections for quick orientation - Dot is aria-hidden (purely decorative) to avoid noise for screen readers - Pairs with the existing unsaved-changes banner below the header Closes #133

- Pressing Ctrl+S (Windows/Linux) or Cmd+S (Mac) opens the diff preview modal - Shortcut is a no-op when the user is actively typing in an input, textarea, select, or contenteditable element — avoids surprising saves mid-edit - Shortcut is also a no-op when saving is in progress or there are validation errors - aria-keyshortcuts attribute on the Save button advertises the shortcut to AT Closes #133

- After a successful save, stores the previous config snapshot in state - 'Undo Last Save' button appears in the header while no new changes exist - Clicking it restores the pre-save draft so the user can review or re-save - Button auto-dismisses after 30 s — uses a useEffect/setTimeout that resets whenever prevSavedConfig changes, so the timer restarts on each new save - Snapshot is also discarded when the selected guild changes, preventing cross-guild undo confusion Closes #133

- ConfigDiff now accepts optional changedSections and onRevertSection props - When both are provided, a strip of section badges + Revert buttons appears in the card header, consistent with the modal's revert UX - Clicking a section badge's RotateCcw button calls onRevertSection(section), which resets that section in the draft back to the last saved value - config-editor passes changedSections and revertSection to the inline ConfigDiff so users can cherry-pick reverts without opening the full diff modal - Props are optional → ConfigDiff stays backward-compatible as a pure display Closes #133

coderabbitai · 2026-03-07T20:23:17Z

📝 Walkthrough

Summary by CodeRabbit

New Features
- Added per-section revert controls to configuration diffs, enabling independent section reverts
- Enhanced undo button display with detailed section change information
Bug Fixes
- Refined keyboard save shortcut (Ctrl/Cmd+S) to prevent unintended saves when typing in form fields
- Undo button now auto-dismisses after 30 seconds

Walkthrough

Removed TASK.md documentation. Enhanced config-diff.tsx with per-section revert UI support via new changedSections and onRevertSection props. Modified config-editor.tsx to wire per-section revert functionality, add 30-second undo auto-dismiss timer, refine keyboard save handling, and improve undo button UX with dynamic section information.

Changes

Cohort / File(s)	Summary
Documentation `TASK.md`	Removed feature specification documentation for Issue `#249` regarding manageable servers visibility.
Dashboard Config Components `web/src/components/dashboard/config-diff.tsx`, `web/src/components/dashboard/config-editor.tsx`	Added per-section revert UI support to ConfigDiff with new props (changedSections, onRevertSection); wired in ConfigEditor with 30-second undo auto-dismiss, refined keyboard save handling to exclude typing in form fields, and dynamic undo button UX showing changed section details.

Possibly related PRs

feat: web dashboard live config editor with real-time bot updates (#31) #83: Extends ConfigDiff and ConfigEditor components with per-section revert props and UI, directly modifying the same components introduced here.
feat(config-editor): diff preview before saving (#133) #217: Modifies config editor and diff flow with per-section revert implementation and modal wiring that overlaps with this PR's diff-preview changes.
feat(web): redesign bot config settings workspace #253: Updates dashboard config editor and ConfigDiff usage signature to handle per-section changedSections and onRevertSection props.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main features added: unsaved indicator, Ctrl+S shortcut, undo functionality, and per-section revert capabilities.
Description check	✅ Passed	The description is well-related to the changeset, detailing all four major features implemented with clear bullet points and correctly closing issue `#133`.
Linked Issues check	✅ Passed	The PR implements all major coding requirements from `#133`: unsaved indicator with section names, Ctrl+S shortcut with form field guard, per-section revert UI, and 30-second undo auto-dismiss functionality.
Out of Scope Changes check	✅ Passed	All changes directly address requirements in `#133`; the only removal is TASK.md documentation which is a cleanup task related to the issue closure.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings (stacked PR)
📝 Generate docstrings (commit on current branch)

🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment
Commit unit tests in branch feat/issue-133

🛠️ Publish Changes: Commit on current branch
🛠️ Publish Changes: Create PR

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.}

Copilot

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

coveralls · 2026-03-07T20:26:52Z

coverage: 87.549%. remained the same
when pulling f3ffa33 on feat/issue-133
into f53f0be on main.

greptile-apps · 2026-03-07T20:28:34Z

Greptile Summary

This PR enhances the config save/diff flow in the dashboard with four improvements: a richer unsaved-changes tooltip showing the count and names of dirty sections, a Ctrl/Cmd+S keyboard shortcut that skips form fields, a 30-second auto-dismissing "Undo last save" button, and per-section revert buttons in the inline diff view. The changes are well-structured and largely fit the existing architecture cleanly.

Key observations:

No tests included — the project rules require new functionality to have tests; revertSection, undoLastSave, the auto-dismiss timer, and the keyboard-shortcut guard all lack coverage.
e.preventDefault() is called for Ctrl/Cmd+S even when the shortcut cannot act (e.g., no changes, validation errors), silently swallowing the keystroke without user feedback.
The per-section RotateCcw revert buttons are destructive but have no confirmation step, making accidental reverts easy to trigger.
TASK.md (a stale task file for a different issue, feat(dashboard): only show manageable servers for mod/admin; fallback to public link for others #249) is correctly cleaned up as part of this PR.

Confidence Score: 3/5

Safe to merge after adding tests; no security or data-integrity regressions introduced.
The logic is correct and the new features are well-isolated, but the project explicitly mandates tests for new functionality and none were provided. The missing coverage for revertSection, undoLastSave, the auto-dismiss timer, and the keyboard-shortcut guard lowers confidence. The per-section revert button also lacks a confirmation step, which is a UX risk for data loss rather than a correctness bug.
web/src/components/dashboard/config-editor.tsx — new callbacks and effects need test coverage before merging.

Sequence Diagram

sequenceDiagram
    participant U as User
    participant CE as ConfigEditor
    participant CD as ConfigDiff
    participant API as /api/guilds/:id/config

    U->>CE: Edit config fields
    CE->>CE: Compute changedSections (useMemo)
    CE->>CE: Update unsaved indicator tooltip

    U->>CE: Ctrl/Cmd+S (outside input/textarea/select)
    CE->>CE: Check hasChanges && !saving && !hasValidationErrors
    CE->>CE: openDiffModal()

    U->>CE: Click Save in diff modal
    CE->>API: PATCH per section (parallel)
    API-->>CE: 200 OK (all sections)
    CE->>CE: setPrevSavedConfig(snapshot)
    CE->>CE: Start 30s auto-dismiss timer
    CE->>API: fetchConfig (reload authoritative state)
    API-->>CE: Updated GuildConfig

    alt User clicks Undo within 30s
        U->>CE: undoLastSave()
        CE->>CE: setDraftConfig(prevSavedConfig.config)
        CE->>CE: setPrevSavedConfig(null) → clears timer
    else 30s elapses
        CE->>CE: setPrevSavedConfig(null) — auto-dismiss
    end

    U->>CD: Click RotateCcw for a section
    CD->>CE: onRevertSection(section)
    CE->>CE: revertSection — restore savedConfig[section] into draftConfig
    CE->>CE: Reset dmStepsRaw if section === 'welcome'

Comments Outside Diff (1)

web/src/components/dashboard/config-editor.tsx, line 401-418 (link)

Missing tests for new functionality

The project rules explicitly require: "New functionality must include tests" and "Tests should cover both happy path and error cases." This PR introduces three distinct new behaviours — the auto-dismiss timer for the undo button, the Ctrl/Cmd+S keyboard shortcut guard, and per-section revert — none of which have corresponding test files in the changeset.

At a minimum, the following should be covered:
- revertSection: happy path (correct section is reverted), dmStepsRaw is reset when section is welcome, no-op when savedConfig is null.
- undoLastSave: auto-dismiss clears prevSavedConfig after 30 s, guild-change guard discards stale snapshot.
- Keyboard shortcut handler: shortcut is a no-op when focus is on INPUT/TEXTAREA/SELECT/contenteditable, and openDiffModal is called when preconditions are met.

_{Last reviewed commit: f3ffa33}

BillChirico · 2026-03-07T22:10:23Z

@coderabbitai review

coderabbitai · 2026-03-07T22:10:28Z

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

web/src/components/dashboard/config-editor.tsx (1)
559-581: 🧹 Nitpick | 🔵 Trivial

Guard should also check isTyping before calling e.preventDefault().

Currently, e.preventDefault() is called after the isTyping check, which is correct. However, there's a subtle issue: if the user is typing and presses Ctrl+S, the function returns early without preventing the default browser behavior. This is actually the intended behavior per the PR objective ("no-op when focused in input/textarea/select"), so this is correct.

One minor edge case: isContentEditable is a property, not a method, and returns a string ('true', 'false', or 'inherit') in some browsers, not a boolean. The truthiness check should still work, but for robustness you could explicitly compare.
♻️ Optional: Explicit contentEditable check
       const isTyping =
         target?.tagName === 'INPUT' ||
         target?.tagName === 'TEXTAREA' ||
         target?.tagName === 'SELECT' ||
-        target?.isContentEditable;
+        target?.isContentEditable === true;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@web/src/components/dashboard/config-editor.tsx` around lines 559 - 581, The
onKeyDown handler currently uses target?.isContentEditable in the isTyping
check, but isContentEditable can be a string in some browsers; update the
isTyping predicate in onKeyDown to explicitly normalize/check contentEditable
(e.g., compare to true or the string 'true') so it reliably detects editable
elements before allowing the function to return (and thus before any call to
e.preventDefault()); locate the isTyping definition inside the onKeyDown
function in config-editor.tsx and change only that boolean expression to a
normalized check for isContentEditable.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@web/src/components/dashboard/config-diff.tsx`:
- Around line 16-24: The prop types should use the ConfigSection union for
stronger type-safety: change changedSections?: string[] to changedSections?:
ConfigSection[] and update onRevertSection?: (section: string) => void to
onRevertSection?: (section: ConfigSection) => void, and add the import for
ConfigSection from the types module; keep both props optional and ensure any
internal usages or callers are updated to pass ConfigSection values (or
cast/validate) so the compiler enforces only valid top-level config keys.

---

Outside diff comments:
In `@web/src/components/dashboard/config-editor.tsx`:
- Around line 559-581: The onKeyDown handler currently uses
target?.isContentEditable in the isTyping check, but isContentEditable can be a
string in some browsers; update the isTyping predicate in onKeyDown to
explicitly normalize/check contentEditable (e.g., compare to true or the string
'true') so it reliably detects editable elements before allowing the function to
return (and thus before any call to e.preventDefault()); locate the isTyping
definition inside the onKeyDown function in config-editor.tsx and change only
that boolean expression to a normalized check for isContentEditable.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro

Run ID: 8a12c1eb-fce1-49d1-947d-215488f283d1

📥 Commits

Reviewing files that changed from the base of the PR and between f53f0be and f3ffa33.

📒 Files selected for processing (3)

TASK.md
web/src/components/dashboard/config-diff.tsx
web/src/components/dashboard/config-editor.tsx

💤 Files with no reviewable changes (1)

TASK.md

📜 Review details

🧰 Additional context used

📓 Path-based instructions (2)

**/*.{js,mjs,jsx,ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

**/*.{js,mjs,jsx,ts,tsx}: Use single quotes for strings in code, double quotes only allowed in JSON files
Always end statements with semicolons
Use 2-space indentation, enforced by Biome

Files:

web/src/components/dashboard/config-diff.tsx
web/src/components/dashboard/config-editor.tsx

web/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Next.js 16 web dashboard uses App Router with Discord OAuth2 authentication, dark/light theme support, and mobile-responsive design

Files:

web/src/components/dashboard/config-diff.tsx
web/src/components/dashboard/config-editor.tsx

🧠 Learnings (7)

📓 Common learnings

Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-07T15:34:56.495Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{js,jsx,ts,tsx} : Config editor save contract must maintain: global save/discard, diff-modal confirmation, per-section PATCH batching, and partial-failure behavior

Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-07T15:34:56.495Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.test.{js,jsx,ts,tsx} : Config editor tests must cover manual-save workspace behavior (not autosave assumptions), category switching, search functionality, and dirty badges

Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-07T15:34:56.495Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{js,jsx,ts,tsx} : Refactor config feature presentation to use reusable `SettingsFeatureCard` pattern with structure: header + master toggle + Basic/Advanced blocks

Learnt from: CR
Repo: VolvoxLLC/volvox-bot PR: 0
File: CLAUDE.md:0-0
Timestamp: 2026-03-07T15:34:56.495Z
Learning: Applies to web/src/components/dashboard/config-workspace/**/*.{js,jsx,ts,tsx} : Config editor must implement metadata-driven config search with cross-category quick jump, focus/scroll targeting, and auto-open advanced sections when search hits advanced controls