Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions src/components/ArticleView/ArticleActionBar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* ArticleActionBar - Fixed bottom banner with Favorite, Translate, AI Summary, Annotate, and Share buttons.
*/

import { useState } from 'react';
import { useState, useCallback, memo } from 'react';
import { useTranslation } from 'react-i18next';
import { Heart, Languages, Sparkles, Loader2, Highlighter, Share2 } from 'lucide-react';

Expand All @@ -19,7 +19,7 @@ interface ArticleActionBarProps {
onToggleAnnotate: () => void;
}

export function ArticleActionBar({
export const ArticleActionBar = memo(function ArticleActionBar({
isFavorite,
isTranslating,
isSummarizing,
Expand All @@ -35,7 +35,7 @@ export function ArticleActionBar({
const [copied, setCopied] = useState(false);
const [shareError, setShareError] = useState(false);

const handleShare = async () => {
const handleShare = useCallback(async () => {
const url = articleLink || window.location.href;
const title = articleTitle || document.title;
if (navigator.share) {
Expand All @@ -58,7 +58,7 @@ export function ArticleActionBar({
setTimeout(() => setShareError(false), 1500);
}
}
};
}, [articleLink, articleTitle]);

return (
<div
Expand Down Expand Up @@ -131,4 +131,4 @@ export function ArticleActionBar({
</div>
</div>
);
}
});
34 changes: 20 additions & 14 deletions src/pages/ArticleDetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,13 @@ function parseContentSegments(html: string): { html: string; text: string }[] {
/** Maximum milliseconds to wait for an AI operation before auto-aborting. */
const AI_OPERATION_TIMEOUT_MS = 60_000;

const ANNOTATION_COLOR_CLASS: Record<string, string> = {
yellow: 'bg-yellow-100 border-yellow-300 dark:bg-yellow-900/30 dark:border-yellow-700',
green: 'bg-green-100 border-green-300 dark:bg-green-900/30 dark:border-green-700',
blue: 'bg-blue-100 border-blue-300 dark:bg-blue-900/30 dark:border-blue-700',
pink: 'bg-pink-100 border-pink-300 dark:bg-pink-900/30 dark:border-pink-700',
};

export function ArticleDetailPage() {
const { article: loaderArticle, feed } = useLoaderData() as ArticleDetailLoaderData;
const navigate = useNavigate();
Expand All @@ -66,6 +73,9 @@ export function ArticleDetailPage() {
const [annotationNote, setAnnotationNote] = useState('');
const [annotationColor, setAnnotationColor] = useState<Annotation['color']>('yellow');
const contentRef = useRef<HTMLDivElement>(null);
// Keep a ref to the latest article for stable callbacks (avoids stale closures)
const articleRef = useRef(article);
articleRef.current = article;
const [translations, setTranslations] = useState<Record<number, string>>({});
const [translatingIndex, setTranslatingIndex] = useState<number>(-1);
const [isTranslating, setIsTranslating] = useState(false);
Expand Down Expand Up @@ -132,22 +142,23 @@ export function ArticleDetailPage() {

// Manual retry: fetch full content from original URL
const handleLoadFullContent = useCallback(async () => {
if (!article.link) return;
if (!articleRef.current.link) return;
setIsLoadingFullContent(true);
setFullContentError(null);
try {
const updated = await fetchAndCacheFullContent(article);
const updated = await fetchAndCacheFullContent(articleRef.current);
setArticle(updated);
} catch {
setFullContentError('Failed to load full article content');
} finally {
setIsLoadingFullContent(false);
}
}, [article]);
}, []);

const sanitizedContent = article.content
? sanitizeHTML(article.content)
: article.summary || '';
const sanitizedContent = useMemo(
() => (article.content ? sanitizeHTML(article.content) : article.summary || ''),
[article.content, article.summary],
);

const segments = useMemo(() => parseContentSegments(sanitizedContent), [sanitizedContent]);

Expand Down Expand Up @@ -294,19 +305,14 @@ export function ArticleDetailPage() {
setAnnotations((prev) => prev.filter((a) => a.id !== id));
}, []);

const annotationColorClass: Record<Annotation['color'], string> = {
yellow: 'bg-yellow-100 border-yellow-300 dark:bg-yellow-900/30 dark:border-yellow-700',
green: 'bg-green-100 border-green-300 dark:bg-green-900/30 dark:border-green-700',
blue: 'bg-blue-100 border-blue-300 dark:bg-blue-900/30 dark:border-blue-700',
pink: 'bg-pink-100 border-pink-300 dark:bg-pink-900/30 dark:border-pink-700',
};
const handleBack = useCallback(() => navigate(-1), [navigate]);

return (
<div className="mx-auto max-w-3xl overflow-x-hidden pb-20">
{/* Navigation */}
<div className="mb-6 flex items-center justify-between">
<button
onClick={() => navigate(-1)}
onClick={handleBack}
className="inline-flex items-center gap-1.5 text-sm text-muted-foreground transition-colors hover:text-foreground"
>
<ArrowLeft className="h-4 w-4" />
Expand Down Expand Up @@ -453,7 +459,7 @@ export function ArticleDetailPage() {
<h3 className="mb-3 text-sm font-semibold text-foreground">Highlights &amp; Annotations</h3>
<ul className="space-y-2">
{annotations.map((ann) => (
<li key={ann.id} className={`rounded-md border px-3 py-2 text-sm ${annotationColorClass[ann.color]}`}>
<li key={ann.id} className={`rounded-md border px-3 py-2 text-sm ${ANNOTATION_COLOR_CLASS[ann.color]}`}>
<p className="font-medium">&ldquo;{ann.selectedText}&rdquo;</p>
{ann.note && <p className="mt-0.5 text-xs text-muted-foreground">{ann.note}</p>}
<button
Expand Down
152 changes: 152 additions & 0 deletions tests/unit/performance/ArticleActionBar.perf.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
/**
* Performance tests for ArticleActionBar React.memo optimization.
* Validates that memo() prevents unnecessary re-renders when parent state changes
* (e.g. during streaming translation/summarization operations).
*/

import { describe, it, expect, vi } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
import { ArticleActionBar } from '@components/ArticleView/ArticleActionBar';

// Mock react-i18next
vi.mock('react-i18next', () => ({
useTranslation: () => ({
t: (key: string) => {
const translations: Record<string, string> = {
favorite: 'Favorite',
favorited: 'Favorited',
};
return translations[key] || key;
},
}),
}));

function createProps(overrides: Partial<Parameters<typeof ArticleActionBar>[0]> = {}) {
return {
isFavorite: false,
isTranslating: false,
isSummarizing: false,
isAnnotating: false,
articleLink: 'https://example.com/article-1',
articleTitle: 'Test Article',
onToggleFavorite: vi.fn(),
onTranslate: vi.fn(),
onSummarize: vi.fn(),
onToggleAnnotate: vi.fn(),
...overrides,
};
}

describe('ArticleActionBar memo behavior', () => {
it('should render all action buttons', () => {
const props = createProps();
render(<ArticleActionBar {...props} />);

expect(screen.getByText('Favorite')).toBeInTheDocument();
expect(screen.getByText('翻译')).toBeInTheDocument();
expect(screen.getByText('AI 总结')).toBeInTheDocument();
expect(screen.getByText('Annotate')).toBeInTheDocument();
expect(screen.getByText('Share')).toBeInTheDocument();
});

it('should not re-render when re-rendered with identical props', () => {
const props = createProps();
const { rerender } = render(<ArticleActionBar {...props} />);

// Re-render parent with same props - memo should prevent re-render of inner component
rerender(<ArticleActionBar {...props} />);
rerender(<ArticleActionBar {...props} />);

// The component should still show correctly
expect(screen.getByText('Favorite')).toBeInTheDocument();
});

it('should re-render when isFavorite prop changes', () => {
const props = createProps({ isFavorite: false });
const { rerender } = render(<ArticleActionBar {...props} />);

expect(screen.getByText('Favorite')).toBeInTheDocument();

rerender(<ArticleActionBar {...createProps({ isFavorite: true })} />);
expect(screen.getByText('Favorited')).toBeInTheDocument();
});

it('should re-render when isTranslating prop changes', () => {
const props = createProps({ isTranslating: false });
const { rerender } = render(<ArticleActionBar {...props} />);

expect(screen.getByText('翻译')).toBeInTheDocument();

rerender(<ArticleActionBar {...createProps({ isTranslating: true })} />);
expect(screen.getByText('停止翻译')).toBeInTheDocument();
});

it('should re-render when isSummarizing prop changes', () => {
const props = createProps({ isSummarizing: false });
const { rerender } = render(<ArticleActionBar {...props} />);

expect(screen.getByText('AI 总结')).toBeInTheDocument();

rerender(<ArticleActionBar {...createProps({ isSummarizing: true })} />);
expect(screen.getByText('停止总结')).toBeInTheDocument();
});

it('should call onToggleFavorite when favorite button is clicked', () => {
const onToggleFavorite = vi.fn();
render(<ArticleActionBar {...createProps({ onToggleFavorite })} />);

fireEvent.click(screen.getByText('Favorite'));
expect(onToggleFavorite).toHaveBeenCalledTimes(1);
});

it('should call onTranslate when translate button is clicked', () => {
const onTranslate = vi.fn();
render(<ArticleActionBar {...createProps({ onTranslate })} />);

fireEvent.click(screen.getByText('翻译'));
expect(onTranslate).toHaveBeenCalledTimes(1);
});

it('should call onSummarize when AI summary button is clicked', () => {
const onSummarize = vi.fn();
render(<ArticleActionBar {...createProps({ onSummarize })} />);

fireEvent.click(screen.getByText('AI 总结'));
expect(onSummarize).toHaveBeenCalledTimes(1);
});

it('should call onToggleAnnotate when annotate button is clicked', () => {
const onToggleAnnotate = vi.fn();
render(<ArticleActionBar {...createProps({ onToggleAnnotate })} />);

fireEvent.click(screen.getByText('Annotate'));
expect(onToggleAnnotate).toHaveBeenCalledTimes(1);
});

it('should show Done label when isAnnotating is true', () => {
render(<ArticleActionBar {...createProps({ isAnnotating: true })} />);
expect(screen.getByText('Done')).toBeInTheDocument();
});

it('should handle rapid re-renders with same callbacks efficiently', () => {
const callbacks = {
onToggleFavorite: vi.fn(),
onTranslate: vi.fn(),
onSummarize: vi.fn(),
onToggleAnnotate: vi.fn(),
};
const props = createProps(callbacks);
const { rerender } = render(<ArticleActionBar {...props} />);

const start = performance.now();
// Simulate 100 rapid re-renders with same props (as would happen during streaming)
for (let i = 0; i < 100; i++) {
rerender(<ArticleActionBar {...props} />);
}
const elapsed = performance.now() - start;

// 100 re-renders should complete in under 500ms thanks to memo
expect(elapsed).toBeLessThan(500);
expect(screen.getByText('Favorite')).toBeInTheDocument();
});
});
Loading
Loading