diff --git a/.chalk/reviews/sessions/pr-13/handoff.md b/.chalk/reviews/sessions/pr-13/handoff.md new file mode 100644 index 0000000..33dbdc3 --- /dev/null +++ b/.chalk/reviews/sessions/pr-13/handoff.md @@ -0,0 +1,46 @@ +# Handoff + +## Scope +- Item: PR #13 — fix: resolve lint errors in widget-renderer.tsx and layout.tsx +- Branch: fix/lint-errors-2026-03-18 +- Commit: b834bc2 +- Goal: Fix lint errors introduced in prior commits and add community/docs files + +## What Changed +- Added CODE_OF_CONDUCT.md (new community document) +- Added CONTRIBUTING.md (new contributor guide) +- Added Makefile with dev/build/lint/clean targets +- Updated README.md Quick Start to use Makefile commands +- Updated asset video link in README.md +- layout.tsx: Migrated from Google Fonts `` tag to Next.js `next/font/google` (Plus_Jakarta_Sans) +- widget-renderer.tsx: Renamed `description` prop to `_description` to suppress unused-var lint warning +- widget-renderer.tsx: Changed `useEffect` → `useLayoutEffect` for iframe srcdoc mutation +- widget-renderer.tsx: Refactored loading phrase reset to use `prevActiveRef` to avoid reset-on-every-render + +## Files Changed +- CODE_OF_CONDUCT.md (+117) +- CONTRIBUTING.md (+207) +- Makefile (+36) +- README.md (+29, -7) +- apps/app/src/app/layout.tsx (+9, -5) +- apps/app/src/components/generative-ui/widget-renderer.tsx (+14, -4) + +## Risk Areas +- **Lint still fails** — the PR claims to fix lint errors but `pnpm lint` exits with 2 errors and 1 warning after the changes +- CONTRIBUTING.md has inconsistent repo URLs (mixes `OpenGenerativeUI` and `open-generative-ui` casing) +- Font variable `--font-plus-jakarta` is set on body but may not be referenced in Tailwind/CSS config +- `useLayoutEffect` calling `setLoaded(false)` + `setHeight(0)` still triggers `react-hooks/set-state-in-effect` lint error + +## Commands Run +- `pnpm lint` — **FAILED** (2 errors, 1 warning in widget-renderer.tsx) + +## Known Gaps +- Lint errors not actually resolved despite PR title claiming they are +- No TypeScript build check run +- No tests added or run + +## Suggested Focus For Reviewers +1. **[BLOCKING]** Lint failure — PR goal unmet; `setIndex(0)` in useEffect (line 417) and `setLoaded(false)` in useLayoutEffect (line 465) still violate `react-hooks/set-state-in-effect` +2. **[BLOCKING]** `_description` warning still present despite renaming +3. CONTRIBUTING.md URL inconsistency (`OpenGenerativeUI` vs `open-generative-ui`) +4. Font variable wiring — confirm `--font-plus-jakarta` is consumed in Tailwind config diff --git a/.chalk/reviews/sessions/pr-13/maintainer-reply.md b/.chalk/reviews/sessions/pr-13/maintainer-reply.md new file mode 100644 index 0000000..ec73233 --- /dev/null +++ b/.chalk/reviews/sessions/pr-13/maintainer-reply.md @@ -0,0 +1,157 @@ +# Maintainer Review Reply — PR #13 + +> fix: resolve lint errors in widget-renderer.tsx and layout.tsx + +--- + +Thanks for the PR! Great that you're tackling the lint errors — this is exactly the kind of housekeeping that keeps the codebase healthy. + +I went through the changes and have a few things to address before we merge. Two of them are blockers because they still trip the lint gate we're trying to fix. + +--- + +### Blocking + +**`react-hooks/set-state-in-effect` — two violations remain in `widget-renderer.tsx`** + +`eslint-plugin-react-hooks@7` introduced a new rule that disallows synchronous `setState` calls directly inside an effect body. The PR currently has two spots that trigger it: + +1. **~line 417** — `setIndex(0)` inside `useEffect`. The intent (reset to 0 when `active` flips true) is right, but the reset needs to happen outside the synchronous effect body. One clean approach: track `active` in a ref and only call `setIndex` inside the `setInterval` callback — start the interval from index 0 and let it cycle from there. + +2. **~lines 465–466** — `setLoaded(false)` and `setHeight(0)` inside `useLayoutEffect`. These could be combined into a single `useReducer` reset, or the reset state stored in refs and derived at render time. + +These two cause CI to exit non-zero, so the PR goal isn't quite met yet. + +--- + +### Minor (non-blocking, but worth cleaning up) + +**~line 431** — `_description` in the destructuring still produces an unused-var warning. The simplest fix is to just omit it from destructuring (`{ title, html }`). The prop can stay in `WidgetRendererProps` for API compatibility — only the binding needs to go. + +**`globals.css` font variable** — `layout.tsx` correctly injects `--font-plus-jakarta` onto `
`, but `globals.css` still references `'Plus Jakarta Sans'` as a hardcoded string instead of `var(--font-plus-jakarta)`. This means Next.js's self-hosted font optimization is wired up but never actually used. Easy one-liner fix: +```css +--font-family: var(--font-plus-jakarta), system-ui, sans-serif; +``` + +--- + +Once the two `set-state-in-effect` issues are sorted, this should be good to go. Happy to help think through the hook refactors if useful — the `useLayoutEffect` one in particular has a few valid approaches depending on how you want to handle the reset. + +--- + ++ Document to Diagram + — powered by CopilotKit +
+Accept the proposed changes?
+ {accepted === null && ( +