docs: improve comparison table color coding and content#58
Conversation
- Color-code cell text green/yellow/red instead of column headers - Agent-Native column header is white; other headers slightly muted gray - Soften red/yellow/green to Tailwind -300 equivalents - Remove 'Non-dev friendly' row - Re-grade cells more fairly (Polished but rigid, Full but slow, etc.) - Update Raw AI Agents copy: Powerful, Instructions and skills - Remove Internal Tools 'Weeks to build' tweak and other copy updates
✅ Deploy Preview for agent-native-fw ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![builder-io-integration[bot]](https://avatars.githubusercontent.com/in/353721?s=60&v=4){kind=small}
There was a problem hiding this comment.
Builder has reviewed your changes and found 3 potential issues.
Review Details
Code Review Summary
This PR does two things: (1) restyling the docs comparison table with per-cell color coding (green/yellow/red text instead of colored column headers), and (2) adding an in-thread search feature (Cmd+F) and thread-level label filtering to the mail template. The docs changes are purely cosmetic and correct. The mail template changes span ~500 lines of new logic across 3 files. Risk: Standard (🟡).
Reviewed by 4 parallel code-review agents with randomized file ordering.
Key Findings
🟡 MEDIUM — Match count desync (HTML emails, line 114): matchCountByMsg strips tags via bodyHtml.replace(/<[^>]+>/g, " ") which leaves <style> and <script> text content intact. The iframe's TreeWalker explicitly skips those nodes. CSS property names like color, text, or bold appearing in <style> blocks will inflate totalMatches, causing navigation to land on non-existent marks. Found by 4/4 agents.
🟡 MEDIUM — HTML active highlight never shown for first match (line 1714): The active-mark effect (deps: [activeLocalIdx, searchTerm]) fires synchronously before the 60ms setTimeout in injectHighlights creates any marks. After the timer fires with new marks, React skips the active-mark effect since deps are unchanged — all HTML matches stay dim-yellow until Next/Prev is pressed. Found by 3/4 agents.
🟡 MEDIUM — Inbox exclusion still per-message (InboxPage.tsx ~line 224): The activeLabel branch correctly uses latest-message thread logic, but the view === "inbox" exclusion branch below still filters per-message. Threads whose latest message has a pinned label survive via older messages, creating ghost threads. Found by 3/4 agents.
🟡 MEDIUM — Pinned system tabs get inbox-only counts (AppLayout.tsx): labelCounts from useEmails("inbox") now fills every pinned tab, including system views. getTotalCount() prefers this map, so Starred/Sent/Drafts show inbox-derived counts instead of real totals. Found by 1/4 agents.
✅ Confirmed Correct
fvs Cmd+F disambiguation works correctly via modifier key checksurroundContentsreverse-order wrapping is safe;normalize()cleanly resets marksuseState-after-useEffectordering is not a runtime bug (closures capture stable setter refs)
Found by running 4 code-review agent calls with randomized file ordering.
Code review by Builder.io
| const searchInputRef = useRef<HTMLInputElement>(null); | ||
|
|
||
| // Match counts per message for in-thread search | ||
| const matchCountByMsg = useMemo(() => { |
There was a problem hiding this comment.
🟡 matchCountByMsg includes <style>/<script> text; iframe TreeWalker skips them
bodyHtml.replace(/<[^>]+>/g, " ") (line 114) leaves CSS/JS text intact, so CSS terms like color or text in <style> blocks inflate totalMatches. The iframe TreeWalker rejects script/style nodes, creating phantom match counts that navigate to non-existent marks. Strip <style>/<script> content before counting to align with the TreeWalker.
How did I do? React with 👍 or 👎 to help me improve.
| { | ||
| key: "f", | ||
| meta: true, | ||
| skipInInput: false, |
There was a problem hiding this comment.
🟡 Cmd+F still opens native browser Find when focus is inside HTML email iframe
Iframe key events are forwarded via window.dispatchEvent() but the original iframe keydown is never canceled. Pressing Cmd+F while focus is inside an HTML email body opens both the custom in-thread search UI and the browser's native Find dialog. Call e.preventDefault() on the original iframe event when the parent handler consumes it.
How did I do? React with 👍 or 👎 to help me improve.
| .filter(([, latest]) => hasLabel(latest)) | ||
| .map(([threadId]) => threadId), | ||
| ); | ||
| return filtered.filter((e) => qualifiedThreadIds.has(e.threadId || e.id)); |
There was a problem hiding this comment.
🟡 Inbox exclusion still filters per-message, not per-thread latest message
The activeLabel branch was updated to latest-message thread logic, but the view === "inbox" exclusion branch below still calls per-message filtering. Threads whose latest message has a pinned label survive via older unpinned messages, creating ghost threads inconsistent with AppLayout's updated thread counts. Apply the same latestByThread map approach used in the activeLabel branch.
How did I do? React with 👍 or 👎 to help me improve.
1 participant
Summary