fix: phase 9+10 refactoring

This commit is contained in:
2026-02-16 07:04:15 +01:00
parent e7c395e1bd
commit 4051fa9333
8 changed files with 295 additions and 128 deletions

View File

@@ -13,6 +13,10 @@ Scope: Reduce high-impact code duplication in production and test-adjacent areas
| 4 | Project mapping + delete guards | ✅ Completed | Guard + mapper extracted and used in target methods. |
| 5 | Metadata sync loop scaffolding | ✅ Completed | `runSyncLoop` extracted and reused by both sync directions. |
| 6 | Shared color utility | ✅ Completed | Shared `getContrastColor` utility is in use. |
| 7 | WXR parse block dedup | ✅ Completed | Shared `parsePubDate` + item base builder in use. |
| 8 | Meta/Tag taxonomy overlap | ✅ Completed | Shared taxonomy normalization helper adopted in both engines. |
| 9 | Renderer tag subscriptions | ✅ Completed | Shared `subscribeToTagEvents` helper with component coverage tests. |
| 10 | Local UI block dedup | ✅ Completed | `AISuggestionsModal` repeated suggestion blocks consolidated with tests. |
### Retired Phases (do not revisit unless regressions appear)
@@ -263,8 +267,8 @@ Move color contrast logic into a shared renderer utility.
3. Phase 3 (finish PostMedia single/batch dedup)
4. ~~Phase 7 (WxrParser repeated parse blocks)~~ ✅ Completed
5. ~~Phase 8 (MetaEngine ↔ TagEngine overlap)~~ ✅ Completed
6. Phase 9 (renderer tag event subscription helper)
7. Phase 10 (local UI repeated blocks in component files)
6. ~~Phase 9 (renderer tag event subscription helper)~~ ✅ Completed
7. ~~Phase 10 (local UI repeated blocks in component files)~~ ✅ Completed
Rationale: complete in-flight high-impact phases first, then address newly detected production hotspots.
@@ -341,6 +345,8 @@ Extract a shared helper module or internal utility used by both engines.
## Phase 9 — Renderer Tag Event Subscription Dedup
Status: ✅ Completed
### Problem
`TagInput` and `TagsView` duplicate tag-event subscription setup/teardown scaffolding.
@@ -356,6 +362,10 @@ Introduce a small shared renderer utility/hook for tag event subscriptions.
- Same runtime behavior and refresh triggers.
- One canonical subscription implementation.
### Progress Check
- Completed: extracted shared `subscribeToTagEvents` helper and adopted it in both `TagInput` and `TagsView`.
- Completed: added renderer component tests that assert subscription/unsubscription behavior for both components.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/renderer/components/TagInput/TagInput.tsx`: 0% statements/functions/branches.
- `src/renderer/components/TagsView/TagsView.tsx`: 0% statements/functions/branches.
@@ -368,6 +378,8 @@ Introduce a small shared renderer utility/hook for tag event subscriptions.
## Phase 10 — Local UI Component Block Dedup (Low Risk)
Status: ✅ Completed
### Problem
Small duplicated blocks exist within individual components (`ProjectSelector`, `CredentialsPanel`, `AISuggestionsModal`, and parts of `App.tsx`).
@@ -383,6 +395,10 @@ Reduce repeated local code with tiny helpers while preserving readability.
- Lower clone count in affected files.
- No UX or behavior changes.
### Progress Check
- Completed: consolidated repeated suggestion item rendering in `AISuggestionsModal` into one shared rendering path.
- Completed: added focused component tests to guard selection/apply and empty-suggestions behavior.
### Coverage & Test Quality (fresh run: `npm run test:coverage`)
- `src/renderer/components/ProjectSelector/ProjectSelector.tsx`: 0% statements/functions/branches.
- `src/renderer/components/CredentialsPanel/CredentialsPanel.tsx`: 0% statements/functions/branches.

View File

@@ -13,6 +13,19 @@ export interface CurrentValues {
caption: string;
}
type SuggestionFieldKey = 'title' | 'alt' | 'caption';
interface SuggestionFieldConfig {
key: SuggestionFieldKey;
label: string;
}
const SUGGESTION_FIELDS: SuggestionFieldConfig[] = [
{ key: 'title', label: 'Title' },
{ key: 'alt', label: 'Alt Text' },
{ key: 'caption', label: 'Caption' },
];
interface AISuggestionsModalProps {
isOpen: boolean;
isLoading: boolean;
@@ -65,6 +78,51 @@ export const AISuggestionsModal: React.FC<AISuggestionsModalProps> = ({
const hasAnySuggestion = suggestions && (suggestions.title || suggestions.alt || suggestions.caption);
const hasAnySelected = useTitle || useAlt || useCaption;
const fieldSelection: Record<SuggestionFieldKey, [boolean, (checked: boolean) => void]> = {
title: [useTitle, setUseTitle],
alt: [useAlt, setUseAlt],
caption: [useCaption, setUseCaption],
};
const renderSuggestionField = (field: SuggestionFieldConfig) => {
if (!suggestions?.[field.key]) {
return null;
}
const [isChecked, setChecked] = fieldSelection[field.key];
const currentValue = currentValues[field.key];
const suggestedValue = suggestions[field.key];
return (
<div key={field.key} className="ai-suggestion-item">
<label className="ai-suggestion-checkbox">
<input
type="checkbox"
checked={isChecked}
onChange={(e) => setChecked(e.target.checked)}
/>
<span className="checkmark"></span>
</label>
<div className="ai-suggestion-content">
<div className="ai-suggestion-label">
{field.label}
{currentValue && (
<span className="ai-suggestion-has-value" title="This field already has a value">
(has existing value)
</span>
)}
</div>
<div className="ai-suggestion-value">{suggestedValue}</div>
{currentValue && (
<div className="ai-suggestion-current">
Current: <em>{currentValue}</em>
</div>
)}
</div>
</div>
);
};
return (
<div className="ai-suggestions-modal-backdrop" onClick={handleBackdropClick}>
<div className="ai-suggestions-modal">
@@ -97,93 +155,7 @@ export const AISuggestionsModal: React.FC<AISuggestionsModalProps> = ({
<p className="ai-suggestions-intro">
Select which AI-generated values to apply. Existing values are preserved by default.
</p>
{suggestions?.title && (
<div className="ai-suggestion-item">
<label className="ai-suggestion-checkbox">
<input
type="checkbox"
checked={useTitle}
onChange={(e) => setUseTitle(e.target.checked)}
/>
<span className="checkmark"></span>
</label>
<div className="ai-suggestion-content">
<div className="ai-suggestion-label">
Title
{currentValues.title && (
<span className="ai-suggestion-has-value" title="This field already has a value">
(has existing value)
</span>
)}
</div>
<div className="ai-suggestion-value">{suggestions.title}</div>
{currentValues.title && (
<div className="ai-suggestion-current">
Current: <em>{currentValues.title}</em>
</div>
)}
</div>
</div>
)}
{suggestions?.alt && (
<div className="ai-suggestion-item">
<label className="ai-suggestion-checkbox">
<input
type="checkbox"
checked={useAlt}
onChange={(e) => setUseAlt(e.target.checked)}
/>
<span className="checkmark"></span>
</label>
<div className="ai-suggestion-content">
<div className="ai-suggestion-label">
Alt Text
{currentValues.alt && (
<span className="ai-suggestion-has-value" title="This field already has a value">
(has existing value)
</span>
)}
</div>
<div className="ai-suggestion-value">{suggestions.alt}</div>
{currentValues.alt && (
<div className="ai-suggestion-current">
Current: <em>{currentValues.alt}</em>
</div>
)}
</div>
</div>
)}
{suggestions?.caption && (
<div className="ai-suggestion-item">
<label className="ai-suggestion-checkbox">
<input
type="checkbox"
checked={useCaption}
onChange={(e) => setUseCaption(e.target.checked)}
/>
<span className="checkmark"></span>
</label>
<div className="ai-suggestion-content">
<div className="ai-suggestion-label">
Caption
{currentValues.caption && (
<span className="ai-suggestion-has-value" title="This field already has a value">
(has existing value)
</span>
)}
</div>
<div className="ai-suggestion-value">{suggestions.caption}</div>
{currentValues.caption && (
<div className="ai-suggestion-current">
Current: <em>{currentValues.caption}</em>
</div>
)}
</div>
</div>
)}
{SUGGESTION_FIELDS.map(renderSuggestionField)}
</div>
)}

View File

@@ -1,6 +1,7 @@
import React, { useState, useEffect, useRef, useCallback } from 'react';
import { showToast } from '../Toast';
import { getContrastColor } from '../../utils/color';
import { subscribeToTagEvents } from '../../utils/tagEventSubscriptions';
import './TagInput.css';
interface TagData {
@@ -54,24 +55,7 @@ export const TagInput: React.FC<TagInputProps> = ({
// Listen for tag changes
useEffect(() => {
const unsubscribers: Array<() => void> = [];
unsubscribers.push(
window.electronAPI?.on('tag:created', () => loadTags()) || (() => {})
);
unsubscribers.push(
window.electronAPI?.on('tag:deleted', () => loadTags()) || (() => {})
);
unsubscribers.push(
window.electronAPI?.on('tag:renamed', () => loadTags()) || (() => {})
);
unsubscribers.push(
window.electronAPI?.on('tags:merged', () => loadTags()) || (() => {})
);
return () => {
unsubscribers.forEach(unsub => unsub());
};
return subscribeToTagEvents(window.electronAPI?.on, loadTags);
}, [loadTags]);
// Filter suggestions based on input

View File

@@ -2,6 +2,7 @@ import React, { useState, useEffect, useCallback } from 'react';
import { useAppStore } from '../../store';
import { showToast } from '../Toast';
import { getContrastColor } from '../../utils/color';
import { subscribeToTagEvents } from '../../utils/tagEventSubscriptions';
import './TagsView.css';
// Types
@@ -179,27 +180,9 @@ export const TagsView: React.FC = () => {
// Listen for tag events
useEffect(() => {
const unsubscribers: Array<() => void> = [];
unsubscribers.push(
window.electronAPI?.on('tag:created', () => loadTags()) || (() => {})
);
unsubscribers.push(
window.electronAPI?.on('tag:updated', () => loadTags()) || (() => {})
);
unsubscribers.push(
window.electronAPI?.on('tag:deleted', () => loadTags()) || (() => {})
);
unsubscribers.push(
window.electronAPI?.on('tag:renamed', () => loadTags()) || (() => {})
);
unsubscribers.push(
window.electronAPI?.on('tags:merged', () => loadTags()) || (() => {})
);
return () => {
unsubscribers.forEach(unsub => unsub());
};
return subscribeToTagEvents(window.electronAPI?.on, loadTags, {
includeUpdated: true,
});
}, [loadTags]);
// Handle tag selection

View File

@@ -0,0 +1,27 @@
type ElectronOn = ((channel: string, callback: (...args: unknown[]) => void) => (() => void) | void) | undefined;
interface SubscribeTagEventsOptions {
includeUpdated?: boolean;
}
const BASE_TAG_EVENTS = ['tag:created', 'tag:deleted', 'tag:renamed', 'tags:merged'] as const;
export function subscribeToTagEvents(
on: ElectronOn,
callback: () => void,
options: SubscribeTagEventsOptions = {}
): () => void {
if (!on) {
return () => {};
}
const channels = options.includeUpdated
? [...BASE_TAG_EVENTS, 'tag:updated']
: BASE_TAG_EVENTS;
const unsubscribers = channels.map((channel) => on(channel, callback) || (() => {}));
return () => {
unsubscribers.forEach((unsubscribe) => unsubscribe());
};
}

View File

@@ -0,0 +1,74 @@
import React from 'react';
import { describe, it, expect, vi } from 'vitest';
import { render, screen, fireEvent } from '@testing-library/react';
import { AISuggestionsModal, type AISuggestions } from '../../../src/renderer/components/AISuggestionsModal/AISuggestionsModal';
const currentValues = {
title: 'Existing title',
alt: 'Existing alt',
caption: '',
};
const baseSuggestions: AISuggestions = {
title: 'Suggested title',
alt: 'Suggested alt',
caption: 'Suggested caption',
};
describe('AISuggestionsModal', () => {
it('shows suggested fields and applies only selected values', () => {
const onConfirm = vi.fn();
render(
<AISuggestionsModal
isOpen
isLoading={false}
suggestions={baseSuggestions}
currentValues={currentValues}
onConfirm={onConfirm}
onClose={vi.fn()}
/>
);
expect(screen.getByText('Suggested title')).toBeTruthy();
expect(screen.getByText('Suggested alt')).toBeTruthy();
expect(screen.getByText('Suggested caption')).toBeTruthy();
const applyButton = screen.getByRole('button', { name: 'Apply Selected' });
const [titleCheckbox, altCheckbox, captionCheckbox] = screen.getAllByRole('checkbox') as HTMLInputElement[];
expect(titleCheckbox.checked).toBe(false);
expect(altCheckbox.checked).toBe(false);
expect(captionCheckbox.checked).toBe(true);
expect(applyButton).not.toBeDisabled();
fireEvent.click(captionCheckbox);
expect(applyButton).toBeDisabled();
fireEvent.click(captionCheckbox);
expect(applyButton).not.toBeDisabled();
fireEvent.click(applyButton);
expect(onConfirm).toHaveBeenCalledTimes(1);
expect(onConfirm).toHaveBeenCalledWith({
caption: 'Suggested caption',
});
});
it('hides apply button when no suggestions are available', () => {
render(
<AISuggestionsModal
isOpen
isLoading={false}
suggestions={{}}
currentValues={{ title: '', alt: '', caption: '' }}
onConfirm={vi.fn()}
onClose={vi.fn()}
/>
);
expect(screen.queryByRole('button', { name: 'Apply Selected' })).toBeNull();
expect(screen.getByText('No suggestions were generated for this image.')).toBeTruthy();
});
});

View File

@@ -0,0 +1,51 @@
import React from 'react';
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { render, act } from '@testing-library/react';
import { TagInput } from '../../../src/renderer/components/TagInput/TagInput';
describe('TagInput subscriptions', () => {
beforeEach(() => {
const onMock = vi.fn((_channel: string, _callback: (...args: unknown[]) => void) => vi.fn());
(window as any).electronAPI = {
...(window as any).electronAPI,
tags: {
getAll: vi.fn().mockResolvedValue([]),
create: vi.fn(),
},
on: onMock,
};
});
it('subscribes to tag refresh events on mount and unsubscribes on unmount', async () => {
const unsubscribeSpies = [vi.fn(), vi.fn(), vi.fn(), vi.fn()];
const onMock = vi
.fn()
.mockImplementationOnce(() => unsubscribeSpies[0])
.mockImplementationOnce(() => unsubscribeSpies[1])
.mockImplementationOnce(() => unsubscribeSpies[2])
.mockImplementationOnce(() => unsubscribeSpies[3]);
(window as any).electronAPI.on = onMock;
const { unmount } = render(<TagInput value={[]} onChange={vi.fn()} />);
await act(async () => {
await Promise.resolve();
});
expect(onMock).toHaveBeenCalledTimes(4);
expect(onMock.mock.calls.map((call) => call[0])).toEqual([
'tag:created',
'tag:deleted',
'tag:renamed',
'tags:merged',
]);
unmount();
unsubscribeSpies.forEach((spy) => {
expect(spy).toHaveBeenCalledTimes(1);
});
});
});

View File

@@ -0,0 +1,60 @@
import React from 'react';
import { describe, it, expect, beforeEach, vi } from 'vitest';
import { render, act } from '@testing-library/react';
import { TagsView } from '../../../src/renderer/components/TagsView/TagsView';
describe('TagsView subscriptions', () => {
beforeEach(() => {
const onMock = vi.fn((_channel: string, _callback: (...args: unknown[]) => void) => vi.fn());
(window as any).electronAPI = {
...(window as any).electronAPI,
tags: {
getWithCounts: vi.fn().mockResolvedValue([]),
getAll: vi.fn().mockResolvedValue([]),
create: vi.fn(),
update: vi.fn(),
delete: vi.fn(),
rename: vi.fn(),
merge: vi.fn(),
syncFromPosts: vi.fn(),
},
on: onMock,
};
});
it('subscribes to tag refresh events including updates and unsubscribes on unmount', async () => {
const unsubscribeSpies = [vi.fn(), vi.fn(), vi.fn(), vi.fn(), vi.fn()];
const onMock = vi
.fn()
.mockImplementationOnce(() => unsubscribeSpies[0])
.mockImplementationOnce(() => unsubscribeSpies[1])
.mockImplementationOnce(() => unsubscribeSpies[2])
.mockImplementationOnce(() => unsubscribeSpies[3])
.mockImplementationOnce(() => unsubscribeSpies[4]);
(window as any).electronAPI.on = onMock;
const { unmount } = render(<TagsView />);
await act(async () => {
await Promise.resolve();
await Promise.resolve();
});
expect(onMock).toHaveBeenCalledTimes(5);
expect(onMock.mock.calls.map((call) => call[0]).sort()).toEqual([
'tag:created',
'tag:deleted',
'tag:renamed',
'tag:updated',
'tags:merged',
]);
unmount();
unsubscribeSpies.forEach((spy) => {
expect(spy).toHaveBeenCalledTimes(1);
});
});
});