refactor(ui): extract shared IconButton from duplicated .icon-btn copies
All checks were successful
deploy / test-frontend (pull_request) Successful in 10m21s
deploy / test-backend (pull_request) Successful in 28m26s
deploy / build-and-push (pull_request) Has been skipped
deploy / deploy (pull_request) Has been skipped

Five files hand-rolled a near-identical 32px `.icon-btn` (same size, hover,
and primary/danger variants). Extract a single IconButton.svelte component
so the treatment lives in one place. Converts the four sites with the
standard 32px form: collections detail, manga edit, upload, and the
chapter-pages editor.

The component takes a `variant` (plain/primary/danger) and spreads any
button attributes (onclick, disabled, aria-label, title, data-testid)
straight through; `type="button"` defaults but a caller can override. The
rendered button keeps the same class, styles, and DOM position, so layout
and behaviour are unchanged — no version bump.

Three icon-button sites are intentionally left out:
- The header (+layout) and home search button are 36px / different radius —
  size outliers that need a size/radius prop before folding in.
- profile/history's copy is being removed in the shared-HistoryList change;
  touching it here would just conflict.

A component (not a global class) avoids colliding with those remaining
local `.icon-btn` definitions.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
MechaCat02
2026-06-25 19:37:08 +02:00
parent dee53fa212
commit 129cb0241d
6 changed files with 156 additions and 153 deletions

View File

@@ -17,6 +17,7 @@
import { onDestroy } from 'svelte'; import { onDestroy } from 'svelte';
import { formatBytes, validateImageFile } from '$lib/upload-validation'; import { formatBytes, validateImageFile } from '$lib/upload-validation';
import Modal from './Modal.svelte'; import Modal from './Modal.svelte';
import IconButton from '$lib/components/IconButton.svelte';
import ArrowUp from '@lucide/svelte/icons/arrow-up'; import ArrowUp from '@lucide/svelte/icons/arrow-up';
import ArrowDown from '@lucide/svelte/icons/arrow-down'; import ArrowDown from '@lucide/svelte/icons/arrow-down';
import Trash2 from '@lucide/svelte/icons/trash-2'; import Trash2 from '@lucide/svelte/icons/trash-2';
@@ -145,36 +146,31 @@
from {p.file.name} · {formatBytes(p.file.size)} from {p.file.name} · {formatBytes(p.file.size)}
</span> </span>
</div> </div>
<button <IconButton
class="icon-btn"
type="button"
onclick={() => movePage(p.id, -1)} onclick={() => movePage(p.id, -1)}
disabled={i === 0} disabled={i === 0}
aria-label="Move {pageLabel(i)} up" aria-label="Move {pageLabel(i)} up"
title="Move up" title="Move up"
> >
<ArrowUp size={16} aria-hidden="true" /> <ArrowUp size={16} aria-hidden="true" />
</button> </IconButton>
<button <IconButton
class="icon-btn"
type="button"
onclick={() => movePage(p.id, 1)} onclick={() => movePage(p.id, 1)}
disabled={i === pages.length - 1} disabled={i === pages.length - 1}
aria-label="Move {pageLabel(i)} down" aria-label="Move {pageLabel(i)} down"
title="Move down" title="Move down"
> >
<ArrowDown size={16} aria-hidden="true" /> <ArrowDown size={16} aria-hidden="true" />
</button> </IconButton>
<button <IconButton
class="icon-btn danger" variant="danger"
type="button"
onclick={() => removePage(p.id)} onclick={() => removePage(p.id)}
aria-label="Remove {pageLabel(i)}" aria-label="Remove {pageLabel(i)}"
title="Remove page" title="Remove page"
data-testid="{testidPrefix}-remove" data-testid="{testidPrefix}-remove"
> >
<Trash2 size={16} aria-hidden="true" /> <Trash2 size={16} aria-hidden="true" />
</button> </IconButton>
{#if p.error} {#if p.error}
<span class="field-error" role="alert">{p.error}</span> <span class="field-error" role="alert">{p.error}</span>
{/if} {/if}
@@ -297,28 +293,6 @@
white-space: nowrap; white-space: nowrap;
} }
.icon-btn {
display: inline-flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
padding: 0;
background: transparent;
color: var(--text-muted);
border: 1px solid transparent;
border-radius: var(--radius-sm);
}
.icon-btn:hover:not(:disabled) {
background: var(--surface-elevated);
color: var(--text);
}
.icon-btn.danger:hover:not(:disabled) {
color: var(--danger);
}
.field-error { .field-error {
grid-column: 1 / -1; grid-column: 1 / -1;
color: var(--danger); color: var(--danger);

View File

@@ -0,0 +1,60 @@
<script lang="ts">
import type { Snippet } from 'svelte';
import type { HTMLButtonAttributes } from 'svelte/elements';
// Shared square icon button — extracted from five near-identical
// `.icon-btn` copies (collections, manga edit, upload, the chapter-pages
// editor) so the size, hover, and variant treatment live in one place.
// Callers pass the lucide icon as the child and any button attributes
// (onclick, disabled, aria-label, title, data-testid) straight through.
type Variant = 'plain' | 'primary' | 'danger';
let {
variant = 'plain',
children,
...rest
}: { variant?: Variant; children: Snippet } & HTMLButtonAttributes = $props();
</script>
<!-- `type="button"` precedes the spread so a caller can still override it
(e.g. a submit button) while the default never submits a form. -->
<button type="button" class="icon-btn {variant}" {...rest}>
{@render children()}
</button>
<style>
.icon-btn {
display: inline-flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
padding: 0;
background: transparent;
color: var(--text-muted);
border: 1px solid transparent;
border-radius: var(--radius-sm);
cursor: pointer;
}
.icon-btn:hover:not(:disabled) {
background: var(--surface-elevated);
color: var(--text);
}
.icon-btn.primary {
background: var(--primary);
color: var(--primary-contrast);
border-color: var(--primary);
}
.icon-btn.primary:hover:not(:disabled) {
background: var(--primary-hover);
border-color: var(--primary-hover);
}
.icon-btn.danger:hover:not(:disabled) {
color: var(--danger);
background: var(--surface-elevated);
}
</style>

View File

@@ -0,0 +1,62 @@
import { describe, it, expect, vi, afterEach } from 'vitest';
import { render, screen, cleanup, fireEvent } from '@testing-library/svelte';
import { createRawSnippet } from 'svelte';
import IconButton from './IconButton.svelte';
afterEach(() => cleanup());
// A stand-in for the lucide icon callers pass as the child.
const icon = createRawSnippet(() => ({
render: () => `<svg data-testid="glyph"></svg>`
}));
describe('IconButton', () => {
it('renders a button containing the icon child', () => {
render(IconButton, { props: { children: icon } });
const btn = screen.getByRole('button');
expect(btn.querySelector('[data-testid="glyph"]')).toBeTruthy();
});
it('defaults to type=button so it never submits a surrounding form by accident', () => {
render(IconButton, { props: { children: icon } });
expect(screen.getByRole('button').getAttribute('type')).toBe('button');
});
it('lets a caller override the type (e.g. submit)', () => {
render(IconButton, { props: { type: 'submit', children: icon } });
expect(screen.getByRole('button').getAttribute('type')).toBe('submit');
});
it('applies the variant as a class (plain by default)', () => {
const { container } = render(IconButton, { props: { children: icon } });
expect(container.querySelector('button.icon-btn.plain')).toBeTruthy();
cleanup();
const { container: c2 } = render(IconButton, {
props: { variant: 'danger', children: icon }
});
expect(c2.querySelector('button.icon-btn.danger')).toBeTruthy();
});
it('forwards onclick', async () => {
const onclick = vi.fn();
render(IconButton, { props: { onclick, children: icon } });
await fireEvent.click(screen.getByRole('button'));
expect(onclick).toHaveBeenCalledOnce();
});
it('forwards arbitrary button attributes (aria-label, title, disabled, data-testid)', () => {
render(IconButton, {
props: {
'aria-label': 'Delete collection',
title: 'Delete',
disabled: true,
'data-testid': 'collection-delete',
children: icon
}
});
const btn = screen.getByTestId('collection-delete');
expect(btn.getAttribute('aria-label')).toBe('Delete collection');
expect(btn.getAttribute('title')).toBe('Delete');
expect((btn as HTMLButtonElement).disabled).toBe(true);
});
});

View File

@@ -12,6 +12,7 @@
import type { Manga } from '$lib/api/client'; import type { Manga } from '$lib/api/client';
import { fileUrl } from '$lib/api/client'; import { fileUrl } from '$lib/api/client';
import MangaCard from '$lib/components/MangaCard.svelte'; import MangaCard from '$lib/components/MangaCard.svelte';
import IconButton from '$lib/components/IconButton.svelte';
import ArrowLeft from '@lucide/svelte/icons/arrow-left'; import ArrowLeft from '@lucide/svelte/icons/arrow-left';
import Pencil from '@lucide/svelte/icons/pencil'; import Pencil from '@lucide/svelte/icons/pencil';
import Check from '@lucide/svelte/icons/check'; import Check from '@lucide/svelte/icons/check';
@@ -148,26 +149,23 @@
{:else} {:else}
<div class="title-row"> <div class="title-row">
<h1 data-testid="collection-name">{collection.name}</h1> <h1 data-testid="collection-name">{collection.name}</h1>
<button <IconButton
type="button"
class="icon-btn"
onclick={startEdit} onclick={startEdit}
aria-label="Edit collection" aria-label="Edit collection"
title="Edit" title="Edit"
data-testid="collection-edit-open" data-testid="collection-edit-open"
> >
<Pencil size={16} aria-hidden="true" /> <Pencil size={16} aria-hidden="true" />
</button> </IconButton>
<button <IconButton
type="button" variant="danger"
class="icon-btn danger"
onclick={onDeleteCollection} onclick={onDeleteCollection}
aria-label="Delete collection" aria-label="Delete collection"
title="Delete" title="Delete"
data-testid="collection-delete" data-testid="collection-delete"
> >
<Trash2 size={16} aria-hidden="true" /> <Trash2 size={16} aria-hidden="true" />
</button> </IconButton>
</div> </div>
{#if collection.description} {#if collection.description}
<p class="description" data-testid="collection-description"> <p class="description" data-testid="collection-description">
@@ -406,25 +404,4 @@
opacity: 1; opacity: 1;
} }
.icon-btn {
display: inline-flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
padding: 0;
background: transparent;
color: var(--text-muted);
border: 1px solid transparent;
border-radius: var(--radius-sm);
}
.icon-btn:hover {
background: var(--surface-elevated);
color: var(--text);
}
.icon-btn.danger:hover {
color: var(--danger);
}
</style> </style>

View File

@@ -10,6 +10,7 @@
import { session } from '$lib/session.svelte'; import { session } from '$lib/session.svelte';
import { formatBytes, validateImageFile } from '$lib/upload-validation'; import { formatBytes, validateImageFile } from '$lib/upload-validation';
import Chip from '$lib/components/Chip.svelte'; import Chip from '$lib/components/Chip.svelte';
import IconButton from '$lib/components/IconButton.svelte';
import Plus from '@lucide/svelte/icons/plus'; import Plus from '@lucide/svelte/icons/plus';
import Trash2 from '@lucide/svelte/icons/trash-2'; import Trash2 from '@lucide/svelte/icons/trash-2';
@@ -190,16 +191,15 @@
maxlength="200" maxlength="200"
data-testid="manga-author-input" data-testid="manga-author-input"
/> />
<button <IconButton
type="button" variant="primary"
class="icon-btn primary"
onclick={addAuthor} onclick={addAuthor}
disabled={!authorDraft.trim()} disabled={!authorDraft.trim()}
aria-label="Add author" aria-label="Add author"
title="Add author" title="Add author"
> >
<Plus size={16} aria-hidden="true" /> <Plus size={16} aria-hidden="true" />
</button> </IconButton>
</div> </div>
</div> </div>
@@ -240,16 +240,15 @@
maxlength="200" maxlength="200"
data-testid="manga-alt-input" data-testid="manga-alt-input"
/> />
<button <IconButton
type="button" variant="primary"
class="icon-btn primary"
onclick={addAltTitle} onclick={addAltTitle}
disabled={!altTitleDraft.trim()} disabled={!altTitleDraft.trim()}
aria-label="Add alternative title" aria-label="Add alternative title"
title="Add alternative title" title="Add alternative title"
> >
<Plus size={16} aria-hidden="true" /> <Plus size={16} aria-hidden="true" />
</button> </IconButton>
</div> </div>
</div> </div>
@@ -270,16 +269,15 @@
src={fileUrl(currentCoverPath)} src={fileUrl(currentCoverPath)}
alt="Current cover" alt="Current cover"
/> />
<button <IconButton
type="button" variant="danger"
class="icon-btn danger"
onclick={markCoverForRemoval} onclick={markCoverForRemoval}
aria-label="Remove cover" aria-label="Remove cover"
title="Remove cover" title="Remove cover"
data-testid="cover-remove" data-testid="cover-remove"
> >
<Trash2 size={16} aria-hidden="true" /> <Trash2 size={16} aria-hidden="true" />
</button> </IconButton>
</div> </div>
{:else if pendingCoverRemoval} {:else if pendingCoverRemoval}
<p class="hint" data-testid="cover-pending-removal"> <p class="hint" data-testid="cover-pending-removal">
@@ -419,39 +417,6 @@
cursor: pointer; cursor: pointer;
} }
.icon-btn {
display: inline-flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
padding: 0;
background: transparent;
color: var(--text-muted);
border: 1px solid transparent;
border-radius: var(--radius-sm);
}
.icon-btn:hover:not(:disabled) {
background: var(--surface-elevated);
color: var(--text);
}
.icon-btn.primary {
background: var(--primary);
color: var(--primary-contrast);
border-color: var(--primary);
}
.icon-btn.primary:hover:not(:disabled) {
background: var(--primary-hover);
border-color: var(--primary-hover);
}
.icon-btn.danger:hover:not(:disabled) {
color: var(--danger);
}
.cover-preview { .cover-preview {
display: flex; display: flex;
align-items: flex-start; align-items: flex-start;

View File

@@ -9,6 +9,7 @@
import ChapterPagesEditor, { import ChapterPagesEditor, {
type PendingPage type PendingPage
} from '$lib/components/ChapterPagesEditor.svelte'; } from '$lib/components/ChapterPagesEditor.svelte';
import IconButton from '$lib/components/IconButton.svelte';
import Plus from '@lucide/svelte/icons/plus'; import Plus from '@lucide/svelte/icons/plus';
import Trash2 from '@lucide/svelte/icons/trash-2'; import Trash2 from '@lucide/svelte/icons/trash-2';
@@ -236,16 +237,15 @@
maxlength="200" maxlength="200"
data-testid="manga-author-input" data-testid="manga-author-input"
/> />
<button <IconButton
type="button" variant="primary"
class="icon-btn primary"
onclick={addAuthor} onclick={addAuthor}
disabled={!authorDraft.trim()} disabled={!authorDraft.trim()}
aria-label="Add author" aria-label="Add author"
title="Add author" title="Add author"
> >
<Plus size={16} aria-hidden="true" /> <Plus size={16} aria-hidden="true" />
</button> </IconButton>
</div> </div>
</div> </div>
@@ -286,16 +286,15 @@
maxlength="200" maxlength="200"
data-testid="manga-alt-input" data-testid="manga-alt-input"
/> />
<button <IconButton
type="button" variant="primary"
class="icon-btn primary"
onclick={addAltTitle} onclick={addAltTitle}
disabled={!altTitleDraft.trim()} disabled={!altTitleDraft.trim()}
aria-label="Add alternative title" aria-label="Add alternative title"
title="Add alternative title" title="Add alternative title"
> >
<Plus size={16} aria-hidden="true" /> <Plus size={16} aria-hidden="true" />
</button> </IconButton>
</div> </div>
</div> </div>
@@ -378,16 +377,15 @@
Failed Failed
{/if} {/if}
</span> </span>
<button <IconButton
type="button" variant="danger"
class="icon-btn danger"
onclick={() => removeChapter(c.id)} onclick={() => removeChapter(c.id)}
aria-label="Remove chapter" aria-label="Remove chapter"
title="Remove chapter" title="Remove chapter"
data-testid="staged-chapter-remove" data-testid="staged-chapter-remove"
> >
<Trash2 size={16} aria-hidden="true" /> <Trash2 size={16} aria-hidden="true" />
</button> </IconButton>
</div> </div>
{#if c.error} {#if c.error}
<p class="field-error" role="alert">{c.error}</p> <p class="field-error" role="alert">{c.error}</p>
@@ -513,39 +511,6 @@
cursor: pointer; cursor: pointer;
} }
.icon-btn {
display: inline-flex;
align-items: center;
justify-content: center;
width: 32px;
height: 32px;
padding: 0;
background: transparent;
color: var(--text-muted);
border: 1px solid transparent;
border-radius: var(--radius-sm);
}
.icon-btn:hover:not(:disabled) {
background: var(--surface-elevated);
color: var(--text);
}
.icon-btn.primary {
background: var(--primary);
color: var(--primary-contrast);
border-color: var(--primary);
}
.icon-btn.primary:hover:not(:disabled) {
background: var(--primary-hover);
border-color: var(--primary-hover);
}
.icon-btn.danger:hover:not(:disabled) {
color: var(--danger);
}
.chapters-header { .chapters-header {
display: flex; display: flex;
align-items: center; align-items: center;