Skip to content

fix: address all PR #202 review findings#204

Merged
github-actions[bot] merged 1 commit into
developfrom
fix/pr202-review-findings
Apr 23, 2026
Merged

fix: address all PR #202 review findings#204
github-actions[bot] merged 1 commit into
developfrom
fix/pr202-review-findings

Conversation

@tomymaritano

@tomymaritano tomymaritano commented Apr 23, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Collaborator

Summary

Fixes all 12 review comments from PR #202 (CodeRabbit + Codex).

  • NoteEditor: Remove note?.id from save effect deps (P2 — prevents false "Saved" flash)
  • exportNote: Reject Windows reserved filenames (CON, PRN, AUX, etc.)
  • installFromUrl: Return slugMismatch in success response for UI awareness
  • SidebarFooter: 60s interval refresh for "Synced Xm ago" staleness
  • UpdateBanner: Fix optional error type + "Download failed: Download failed" duplication
  • PluginsSection: Add slug to marketplace response validation
  • Section.module.css: Keyframe pluginSpin → plugin-spin (stylelint)
  • Toast: Remove nested aria-live from container (spec compliance)
  • tsconfig.json: jsx: "preserve" for Next.js (was "react-jsx")
  • .env.example: Reference to CLAUDE.md for setup docs

Test plan

  • pnpm typecheck — 17/17 pass
  • pnpm test — 16/16 pass

🤖 Generated with Claude Code

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Fixed handling of Windows reserved filenames in exports
    • Download failures now display specific error messages instead of generic text
    • Improved validation when installing marketplace plugins
    • Sync timestamp display now updates periodically
    • Enhanced accessibility for toast notifications

@tomymaritano @claude
fix: address all PR #202 review findings (CodeRabbit + Codex)
bc35c9d 
- NoteEditor: remove note?.id from save effect deps (prevents false flash)
- exportNote: reject Windows reserved filenames (CON, PRN, etc.)
- installFromUrl: return slugMismatch in success response for UI warning
- SidebarFooter: 60s interval to refresh "Synced Xm ago" text
- UpdateBanner: fix error type + avoid duplicate "Download failed" message
- PluginsSection: add slug to marketplace validation filter
- Section.module.css: rename keyframe to kebab-case (plugin-spin)
- Toast: remove nested aria-live from container (keep on individual toasts)
- apps/web/tsconfig.json: jsx "preserve" for Next.js compatibility
- .env.example: add reference to CLAUDE.md for setup docs

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@vercel

vercel Bot commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
readide Error Error Apr 23, 2026 8:10pm

Request Review

@github-actions github-actions Bot enabled auto-merge (squash) April 23, 2026 20:09
@coderabbitai

coderabbitai Bot commented Apr 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

The pull request introduces incremental improvements across the desktop application and web configuration: Windows filename sanitization, enhanced error reporting in download operations with API bridge updates, stricter plugin marketplace validation, accessibility adjustments to toast notifications, UI refresh mechanisms for relative timestamps, and a TypeScript compilation setting change.

Changes

Cohort / File(s) Summary
Configuration & Setup
.env.example, apps/web/tsconfig.json		 Added developer setup guidance comment and updated JSX compilation mode from react-jsx to preserve.
Error Handling & API Bridge
apps/desktop/src/main/index.ts, apps/desktop/src/preload/index.ts, apps/desktop/src/renderer/components/UpdateBanner.tsx		 Enhanced download failure responses with captured error messages; updated ReadiedAPI.updates.startDownload() return type to include optional error field; improved error message sourcing from download results instead of hardcoded fallback.
Renderer Components & Validation
apps/desktop/src/renderer/components/NoteEditor.tsx, apps/desktop/src/renderer/components/sidebar/SidebarFooter.tsx, apps/desktop/src/renderer/pages/settings/sections/PluginsSection.tsx		 Modified "saved" indicator effect dependency to re-run only on isDirty changes; added 60-second interval to refresh relative time display for sync history; implemented stricter marketplace plugin validation requiring slug field to be a string.
Filename & UI Polish
apps/desktop/src/main/index.ts (secondary), apps/desktop/src/renderer/pages/settings/sections/Section.module.css, apps/desktop/src/renderer/ui/primitives/Toast.tsx		 Added detection and prefixing of Windows-reserved device filenames; renamed CSS animation keyframe from pluginSpin to plugin-spin; removed aria-live attribute from Toaster container to rely on individual toast notification attributes.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'fix: address all PR #202 review findings' is vague and generic, using non-descriptive phrasing that doesn't convey specific information about the actual changes made. Consider a more descriptive title that highlights the primary changes, such as 'fix: update save effect dependencies, file validation, and UI text refresh' or similar.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

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

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/pr202-review-findings

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.

@github-actions github-actions Bot merged commit b0700b4 into develop Apr 23, 2026
13 of 15 checks passed
@tomymaritano tomymaritano mentioned this pull request Apr 23, 2026
Merged
1 task
tomymaritano added a commit that referenced this pull request Apr 23, 2026
@tomymaritano @claude @dependabot
fix: sync main into develop for clean release (#205)
b5d2733 
## Summary
Merges main into develop to resolve all conflicts so PR #202 (release)
can merge cleanly.

All 5 conflicts resolved keeping develop's version (which has all review
fixes from PRs #199-#204).

## Test plan
- [x] `pnpm typecheck` — 17/17 pass

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Signed-off-by: dependabot[bot] <support@github.com>
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
@tomymaritano tomymaritano deleted the fix/pr202-review-findings branch April 23, 2026 20:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

app:desktop app:web size/S

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@tomymaritano