From 4cd15bb8a04bf8df3fb292796a8f32d7533cacdc Mon Sep 17 00:00:00 2001 From: Adam Mathes Date: Sun, 15 Feb 2026 15:57:54 -0800 Subject: Optimize frontend with memoized FeedItem and efficient IntersectionObserver --- .agent/rules/before-commit.md | 7 ++ .agent/rules/before-ticket-close.md | 5 ++ .thicket/tickets.jsonl | 2 + frontend/src/components/FeedItem.test.tsx | 48 +++++------- frontend/src/components/FeedItem.tsx | 75 ++++++------------ frontend/src/components/FeedItems.test.tsx | 58 +++++--------- frontend/src/components/FeedItems.tsx | 122 ++++++++++++++++++----------- performance_analysis.md | 35 +++++++++ 8 files changed, 186 insertions(+), 166 deletions(-) create mode 100644 .agent/rules/before-commit.md create mode 100644 .agent/rules/before-ticket-close.md create mode 100644 performance_analysis.md diff --git a/.agent/rules/before-commit.md b/.agent/rules/before-commit.md new file mode 100644 index 0000000..ddf97e8 --- /dev/null +++ b/.agent/rules/before-commit.md @@ -0,0 +1,7 @@ +--- +trigger: always_on +--- + +Before git commit of any changes, run `make test` to ensure tests pass. + +And ensure you run `make all` to build the production assets. Those need to be checked in after any UI changes! \ No newline at end of file diff --git a/.agent/rules/before-ticket-close.md b/.agent/rules/before-ticket-close.md new file mode 100644 index 0000000..1e58b11 --- /dev/null +++ b/.agent/rules/before-ticket-close.md @@ -0,0 +1,5 @@ +--- +trigger: always_on +--- + +Before closing a ticket with thicket close -- be sure to run make test and check in any changes! \ No newline at end of file diff --git a/.thicket/tickets.jsonl b/.thicket/tickets.jsonl index 10a1c2c..d5d256d 100644 --- a/.thicket/tickets.jsonl +++ b/.thicket/tickets.jsonl @@ -48,6 +48,7 @@ {"id":"NK-ca9t70","title":"Vanilla JS: Add Feed UI","description":"Add UI to add a new feed by URL in vanilla JS prototype.","type":"feature","status":"closed","priority":2,"labels":null,"assignee":"","created":"2026-02-14T04:47:41.764330544Z","updated":"2026-02-14T04:47:41.764330544Z"} {"id":"NK-chns2b","title":"reach parity between vanilla js and react v2 ui","description":"Continue implementing the vanilla js one with minimal overhad/depdnencies to be fast and lean. Make sure there are tests and rely on the v2 ui and legacy version as references.","type":"epic","status":"closed","priority":1,"labels":null,"assignee":"","created":"2026-02-14T04:45:06.813453353Z","updated":"2026-02-14T04:45:06.813453353Z"} {"id":"NK-ck4co9","title":"Refactor E2E tests to use page objects","description":"The E2E tests are getting complex. Refactor them to use the Page Object Model pattern for better maintainability.","type":"task","status":"open","priority":4,"labels":null,"assignee":"","created":"2026-02-15T02:21:34.96843041Z","updated":"2026-02-15T19:14:31.660189629Z"} +{"id":"NK-d2be57","title":"Persist sidebar state across reloads","description":"Currently sidebar state resets on reload. It should persist in localStorage like the theme.","type":"feature","status":"open","priority":2,"labels":null,"assignee":"","created":"2026-02-15T22:23:06.847360465Z","updated":"2026-02-15T22:23:06.847360465Z"} {"id":"NK-d4c8jv","title":"Vanilla JS Parity: Read/Star/Filter","description":"Implement read/unread toggle, star/unstar, and special filters (All, Unread, Starred) in vanilla JS prototype.","type":"feature","status":"closed","priority":1,"labels":null,"assignee":"","created":"2026-02-14T04:46:32.113504545Z","updated":"2026-02-14T04:47:46.412290355Z"} {"id":"NK-dbcl6t","title":"Create Python Compliance Suite","description":"","type":"task","status":"closed","priority":2,"labels":null,"assignee":"","created":"2026-02-15T00:21:53.997204693Z","updated":"2026-02-15T00:44:41.52830766Z"} {"id":"NK-doss0v","title":"v2 ui: change title fonts to Helvetica Neue","description":"to match style in legacy change font to match Helventic Neue where applicable","type":"bug","status":"closed","priority":0,"labels":null,"assignee":"","created":"2026-02-14T06:27:57.270935467Z","updated":"2026-02-14T06:31:42.798620609Z"} @@ -153,6 +154,7 @@ {"id":"NK-dda9zfr","from_ticket_id":"NK-lrew5z","to_ticket_id":"NK-mwf9q2","type":"created_from","created":"2026-02-13T18:04:57.273164732Z"} {"id":"NK-de65jjz","from_ticket_id":"NK-p0nfoi","to_ticket_id":"NK-r8rs7m","type":"created_from","created":"2026-02-15T16:49:31.043201298Z"} {"id":"NK-dew7hvb","from_ticket_id":"NK-tw0nga","to_ticket_id":"NK-59kbij","type":"created_from","created":"2026-02-13T15:01:33.825547908Z"} +{"id":"NK-dfercff","from_ticket_id":"NK-d2be57","to_ticket_id":"NK-hy162w","type":"created_from","created":"2026-02-15T22:23:06.888593551Z"} {"id":"NK-dffwhjf","from_ticket_id":"NK-2t5ijy","to_ticket_id":"NK-lrew5z","type":"created_from","created":"2026-02-13T18:11:47.471931543Z"} {"id":"NK-dfyyk6k","from_ticket_id":"NK-hj6f9p","to_ticket_id":"NK-kra45a","type":"created_from","created":"2026-02-15T01:04:54.417714174Z"} {"id":"NK-dgbrb79","from_ticket_id":"NK-9hx0y7","to_ticket_id":"NK-t0nmbj","type":"created_from","created":"2026-02-13T05:44:01.556027956Z"} diff --git a/frontend/src/components/FeedItem.test.tsx b/frontend/src/components/FeedItem.test.tsx index 1c51dc3..ab2ca45 100644 --- a/frontend/src/components/FeedItem.test.tsx +++ b/frontend/src/components/FeedItem.test.tsx @@ -27,66 +27,54 @@ describe('FeedItem Component', () => { render(); expect(screen.getByText('Test Item')).toBeInTheDocument(); expect(screen.getByText(/Test Feed/)).toBeInTheDocument(); - // Check for relative time or date formatting? For now just check it renders }); - it('toggles star status', async () => { - vi.mocked(global.fetch).mockResolvedValueOnce({ ok: true, json: async () => ({}) } as Response); - - render(); + it('calls onToggleStar when star clicked', () => { + const onToggleStar = vi.fn(); + render(); const starBtn = screen.getByTitle('Star'); - expect(starBtn).toHaveTextContent('★'); fireEvent.click(starBtn); - // Optimistic update - expect(await screen.findByTitle('Unstar')).toHaveTextContent('★'); - - expect(global.fetch).toHaveBeenCalledWith( - '/api/item/1', - expect.objectContaining({ - method: 'PUT', - body: JSON.stringify({ - _id: 1, - read: false, - starred: true, - }), - }) - ); + expect(onToggleStar).toHaveBeenCalledWith(mockItem); }); it('updates styling when read state changes', () => { const { rerender } = render(); const link = screen.getByText('Test Item'); - // Initial state: unread (bold) - // Note: checking computed style might be flaky in jsdom, but we can check the class on the parent const listItem = link.closest('li'); expect(listItem).toHaveClass('unread'); expect(listItem).not.toHaveClass('read'); - // Update prop to read rerender(); - - // Should now be read expect(listItem).toHaveClass('read'); expect(listItem).not.toHaveClass('unread'); }); - it('loads full content', async () => { + it('loads full content and calls onUpdate', async () => { + const onUpdate = vi.fn(); vi.mocked(global.fetch).mockResolvedValueOnce({ ok: true, - json: async () => ({ ...mockItem, full_content: '

Full Content Loaded

' }), + json: async () => ({ full_content: '

Full Content Loaded

' }), } as Response); - render(); + const { rerender } = render(); const scrapeBtn = screen.getByTitle('Load Full Content'); fireEvent.click(scrapeBtn); await waitFor(() => { - expect(screen.getByText('Full Content Loaded')).toBeInTheDocument(); + expect(global.fetch).toHaveBeenCalledWith('/api/item/1', expect.anything()); + }); + + await waitFor(() => { + expect(onUpdate).toHaveBeenCalledWith(expect.objectContaining({ + full_content: '

Full Content Loaded

' + })); }); - expect(global.fetch).toHaveBeenCalledWith('/api/item/1', expect.anything()); + // Simulate parent updating prop + rerender(Full Content Loaded

' }} onUpdate={onUpdate} />); + expect(screen.getByText('Full Content Loaded')).toBeInTheDocument(); }); }); diff --git a/frontend/src/components/FeedItem.tsx b/frontend/src/components/FeedItem.tsx index ac142dc..865c080 100644 --- a/frontend/src/components/FeedItem.tsx +++ b/frontend/src/components/FeedItem.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect } from 'react'; +import { useState, memo } from 'react'; import type { Item } from '../types'; import './FeedItem.css'; @@ -6,54 +6,26 @@ import { apiFetch } from '../utils'; interface FeedItemProps { item: Item; + onToggleStar?: (item: Item) => void; + onUpdate?: (item: Item) => void; } -export default function FeedItem({ item: initialItem }: FeedItemProps) { - const [item, setItem] = useState(initialItem); +const FeedItem = memo(function FeedItem({ item, onToggleStar, onUpdate }: FeedItemProps) { const [loading, setLoading] = useState(false); - useEffect(() => { - setItem(initialItem); - }, [initialItem]); + // We rely on props.item for data. + // If we fetch full content, we notify the parent via onUpdate. - const toggleStar = () => { - updateItem({ ...item, starred: !item.starred }); - }; - - const updateItem = (newItem: Item) => { - setLoading(true); - // Optimistic update - const previousItem = item; - setItem(newItem); - - apiFetch(`/api/item/${newItem._id}`, { - method: 'PUT', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - _id: newItem._id, - read: newItem.read, - starred: newItem.starred, - }), - }) - .then((res) => { - if (!res.ok) { - throw new Error('Failed to update item'); - } - return res.json(); - }) - .then(() => { - // Confirm with server response if needed, but for now we trust the optimistic update - // or we could setItem(updated) if the server returns the full object - setLoading(false); - }) - .catch((err) => { - console.error('Error updating item:', err); - // Revert on error - setItem(previousItem); - setLoading(false); - }); + const handleToggleStar = (e: React.MouseEvent) => { + e.stopPropagation(); + if (onToggleStar) { + onToggleStar(item); + } else { + // Fallback if no handler passed (backward compat or isolated usage) + // But really we should rely on parent. + // For now, let's keep the optimistic local update logic if we were standalone, + // but since we are optimizing, we assume parent handles it. + } }; const loadFullContent = (e: React.MouseEvent) => { @@ -65,7 +37,11 @@ export default function FeedItem({ item: initialItem }: FeedItemProps) { return res.json(); }) .then((data) => { - setItem({ ...item, ...data }); + // Merge the new data (full_content) into the item and notify parent + const newItem = { ...item, ...data }; + if (onUpdate) { + onUpdate(newItem); + } setLoading(false); }) .catch((err) => { @@ -81,10 +57,7 @@ export default function FeedItem({ item: initialItem }: FeedItemProps) { {item.title || '(No Title)'}