refactor(ui): extract shared IconButton from duplicated .icon-btn copies
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:
@@ -17,6 +17,7 @@
|
||||
import { onDestroy } from 'svelte';
|
||||
import { formatBytes, validateImageFile } from '$lib/upload-validation';
|
||||
import Modal from './Modal.svelte';
|
||||
import IconButton from '$lib/components/IconButton.svelte';
|
||||
import ArrowUp from '@lucide/svelte/icons/arrow-up';
|
||||
import ArrowDown from '@lucide/svelte/icons/arrow-down';
|
||||
import Trash2 from '@lucide/svelte/icons/trash-2';
|
||||
@@ -145,36 +146,31 @@
|
||||
from {p.file.name} · {formatBytes(p.file.size)}
|
||||
</span>
|
||||
</div>
|
||||
<button
|
||||
class="icon-btn"
|
||||
type="button"
|
||||
<IconButton
|
||||
onclick={() => movePage(p.id, -1)}
|
||||
disabled={i === 0}
|
||||
aria-label="Move {pageLabel(i)} up"
|
||||
title="Move up"
|
||||
>
|
||||
<ArrowUp size={16} aria-hidden="true" />
|
||||
</button>
|
||||
<button
|
||||
class="icon-btn"
|
||||
type="button"
|
||||
</IconButton>
|
||||
<IconButton
|
||||
onclick={() => movePage(p.id, 1)}
|
||||
disabled={i === pages.length - 1}
|
||||
aria-label="Move {pageLabel(i)} down"
|
||||
title="Move down"
|
||||
>
|
||||
<ArrowDown size={16} aria-hidden="true" />
|
||||
</button>
|
||||
<button
|
||||
class="icon-btn danger"
|
||||
type="button"
|
||||
</IconButton>
|
||||
<IconButton
|
||||
variant="danger"
|
||||
onclick={() => removePage(p.id)}
|
||||
aria-label="Remove {pageLabel(i)}"
|
||||
title="Remove page"
|
||||
data-testid="{testidPrefix}-remove"
|
||||
>
|
||||
<Trash2 size={16} aria-hidden="true" />
|
||||
</button>
|
||||
</IconButton>
|
||||
{#if p.error}
|
||||
<span class="field-error" role="alert">{p.error}</span>
|
||||
{/if}
|
||||
@@ -297,28 +293,6 @@
|
||||
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 {
|
||||
grid-column: 1 / -1;
|
||||
color: var(--danger);
|
||||
|
||||
60
frontend/src/lib/components/IconButton.svelte
Normal file
60
frontend/src/lib/components/IconButton.svelte
Normal 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>
|
||||
62
frontend/src/lib/components/IconButton.svelte.test.ts
Normal file
62
frontend/src/lib/components/IconButton.svelte.test.ts
Normal 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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user