Skip to content

🎨 Palette: Improve file upload UX with dynamic icons and size formatting #604

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
anchapin wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🎨 Palette: Improve file upload UX with dynamic icons and size formatting #604

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
36 changes: 29 additions & 7 deletions frontend/src/components/ConversionUpload/ConversionUploadEnhanced.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import userEvent from '@testing-library/user-event';
import { ConversionUploadEnhanced } from './ConversionUploadEnhanced';
import { describe, test, expect, vi } from 'vitest';
import { convertMod } from '../../services/api';
import { ProgressProvider } from '../../contexts/ProgressContext';

// Mock the API calls
vi.mock('../../services/api', () => ({
Expand All @@ -17,16 +18,21 @@ vi.mock('../../services/api', () => ({
// Mock the WebSocket service
vi.mock('../../services/websocket', () => ({
createConversionWebSocket: vi.fn(() => ({
onStatus: vi.fn(),
onMessage: vi.fn(),
onStatus: vi.fn(() => vi.fn()),
onMessage: vi.fn(() => vi.fn()),
connect: vi.fn(),
destroy: vi.fn(),
disconnect: vi.fn(),
})),
}));

describe('ConversionUploadEnhanced Accessibility', () => {
test('Smart Assumptions info button has correct accessibility attributes', () => {
render(<ConversionUploadEnhanced />);
render(
<ProgressProvider>
<ConversionUploadEnhanced />
</ProgressProvider>
);

// Find the info button
const infoButton = screen.getByText('?');
Expand All @@ -52,7 +58,11 @@ describe('ConversionUploadEnhanced Accessibility', () => {

test('Remove file button has accessible name', async () => {
const user = userEvent.setup();
render(<ConversionUploadEnhanced />);
render(
<ProgressProvider>
<ConversionUploadEnhanced />
</ProgressProvider>
);

// Upload a file
const file = new File(['dummy content'], 'test-mod.jar', { type: 'application/java-archive' });
Expand All @@ -74,7 +84,11 @@ describe('ConversionUploadEnhanced Accessibility', () => {

test('Error message has role="alert"', async () => {
const user = userEvent.setup();
render(<ConversionUploadEnhanced />);
render(
<ProgressProvider>
<ConversionUploadEnhanced />
</ProgressProvider>
);

// Trigger an error via invalid URL
const urlInput = screen.getByPlaceholderText(/curseforge/i);
Expand All @@ -89,14 +103,22 @@ describe('ConversionUploadEnhanced Accessibility', () => {
});

test('URL input has an accessible label', () => {
render(<ConversionUploadEnhanced />);
render(
<ProgressProvider>
<ConversionUploadEnhanced />
</ProgressProvider>
);
const urlInput = screen.getByLabelText('Modpack URL');
expect(urlInput).toBeInTheDocument();
});

test('Button shows spinner when processing', async () => {
const user = userEvent.setup();
render(<ConversionUploadEnhanced />);
render(
<ProgressProvider>
<ConversionUploadEnhanced />
</ProgressProvider>
);

// Check initial button text
const submitButton = screen.getByText('Upload & Convert');
Expand Down
15 changes: 11 additions & 4 deletions frontend/src/components/ConversionUpload/ConversionUploadEnhanced.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
ConversionStatusEnum
} from '../../types/api';
import ConversionProgress from '../ConversionProgress/ConversionProgress';
import { formatFileSize } from '../../utils/formatters';
import './ConversionUpload.css';

// Configuration constants
Expand Down Expand Up @@ -410,6 +411,12 @@ export const ConversionUploadEnhanced: React.FC<ConversionUploadProps> = ({
const isCancelled = currentStatus?.status === ConversionStatusEnum.CANCELLED;
const isFinished = isCompleted || isFailed || isCancelled;

const getFileIcon = (fileName: string) => {
if (fileName.toLowerCase().endsWith('.jar')) return '☕';
if (fileName.toLowerCase().endsWith('.zip')) return '🗜️';
return '📦';
};

return (
<div className="conversion-upload">
<h2>Convert Your Modpack</h2>
Expand Down Expand Up @@ -442,10 +449,10 @@ export const ConversionUploadEnhanced: React.FC<ConversionUploadProps> = ({
</div>
) : selectedFile ? (
<div className="file-preview">
<div className="file-icon">📦</div>
<div className="file-icon" aria-hidden="true">{getFileIcon(selectedFile.name)}</div>
<div className="file-info">
<div className="file-name">{selectedFile.name}</div>
<div className="file-size">{(selectedFile.size / 1024 / 1024).toFixed(2)} MB</div>
<div className="file-size">{formatFileSize(selectedFile.size)}</div>
<div className="status">{getStatusMessage()}</div>
</div>
<button
Expand All @@ -463,8 +470,8 @@ export const ConversionUploadEnhanced: React.FC<ConversionUploadProps> = ({
</div>
) : (
<div className="upload-prompt initial-prompt">
<div className="upload-icon-large">☁️</div>
<h3>Drag & drop your modpack here</h3>
<div className="upload-icon-large" aria-hidden="true">☁️</div>
<h3>{isDragActive ? "Drop file to upload 📂" : "Drag & drop your modpack here"}</h3>
Comment on lines 414 to 474
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new dynamic file icon feature (getFileIcon function) and drag feedback text changes lack test coverage. The existing tests have been updated to wrap components in ProgressProvider, but no tests verify that: 1) the coffee emoji (☕) appears for .jar files, 2) the clamp emoji (🗜️) appears for .zip files, 3) the default package emoji (📦) appears for other files, or 4) the drag-active text "Drop file to upload 📂" appears when dragging files. Consider adding tests to verify these new UX improvements work correctly.

Copilot uses AI. Check for mistakes.
<button
type="button"
className="browse-button"
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/utils/formatters.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export const formatFileSize = (bytes: number): string => {
if (bytes === 0) return '0 Bytes';
const k = 1024;
const sizes = ['Bytes', 'KB', 'MB', 'GB', 'TB'];
const i = Math.floor(Math.log(bytes) / Math.log(k));
return parseFloat((bytes / Math.pow(k, i)).toFixed(2)) + ' ' + sizes[i];
Comment on lines +1 to +6
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatFileSize function doesn't handle negative byte values. While file sizes should never be negative in practice, defensive programming suggests either validating the input (throwing an error or returning an error message like "Invalid size") or documenting that the function expects non-negative values. Currently, a negative value would produce an incorrect result like "-1.00 KB" due to the Math.log calculation with negative numbers.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new formatFileSize utility function lacks test coverage. Consider adding unit tests to verify edge cases such as: zero bytes, very small files (< 1KB), boundary values between units (e.g., 1023 bytes vs 1024 bytes), large files (GB and TB ranges), and the formatting precision (2 decimal places). This is especially important since the function will likely be reused across the codebase to replace duplicated implementations.

Suggested change
return parseFloat((bytes / Math.pow(k, i)).toFixed(2)) + ' ' + sizes[i];
const value = (bytes / Math.pow(k, i)).toFixed(2);
return `${value} ${sizes[i]}`;

Copilot uses AI. Check for mistakes.
};
Comment on lines +1 to +7
Copy link

Copilot AI Feb 23, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The formatFileSize function is duplicated across the codebase in at least 5 different locations (ConversionAssetDetails.tsx, ConversionAssetsUpload.tsx, ConversionHistory/utils.ts, urlParser.ts, and now formatters.ts). This creates a maintainability issue where bug fixes or improvements need to be applied in multiple places. Consider consolidating all instances to use the new utility in formatters.ts and removing the duplicated implementations. This would be especially beneficial since the implementations are slightly different - some use 'Bytes' while others use 'B', and the ConversionHistory version always shows MB regardless of size.

Copilot uses AI. Check for mistakes.
2 changes: 1 addition & 1 deletion pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Binary file added verification_jar.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added verification_zip.png
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.