Skip to content

Display file sizes in package file browser#255

Merged
simonw merged 1 commit intomainfrom
claude/add-file-sizes-tzQ6F
Mar 20, 2026
Merged

Display file sizes in package file browser#255
simonw merged 1 commit intomainfrom
claude/add-file-sizes-tzQ6F

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Mar 20, 2026

zip-wheel-explorer - have it show the file size in bytes/KB/MB (whichever is nested to display) next to each filename in the listing

Summary

Added file size display to the package file browser, showing the size of each file in the archive in human-readable format (bytes, KB, or MB).

Key Changes

  • Added .file-size CSS class for styling file size display with muted color and smaller font
  • Modified file list rendering to display file name and size as separate elements within each file item
  • Implemented formatFileSize() function to convert byte counts to human-readable format (bytes, KB, MB)
  • Updated extractFiles() to capture the byte length of each file and include it in the file object

Implementation Details

  • File sizes are displayed inline next to the filename with white-space: nowrap to prevent wrapping
  • Size formatting uses standard thresholds: 1 MB = 1048576 bytes, 1 KB = 1024 bytes
  • File sizes are extracted as uint8array during ZIP parsing to get accurate byte lengths
  • The size display uses a muted gray color (#999) and smaller font size (0.75rem) to maintain visual hierarchy

https://claude.ai/code/session_01Rh44SzVc2P6pixRfeA1EKN

Shows human-readable file sizes (bytes/KB/MB) next to each filename
in the file listing, using the most appropriate unit for each file.

https://claude.ai/code/session_01Rh44SzVc2P6pixRfeA1EKN
@claude
Copy link
Contributor

claude bot commented Mar 20, 2026

Claude finished @simonw's task —— View job


Code Review for PR #255: Display file sizes in package file browser

Review Tasks:

  • Read and analyze the modified zip-wheel-explorer.html file
  • Evaluate code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage needs
  • Provide constructive feedback and recommendations

Review Summary

This PR successfully adds file size display functionality to the package file browser. The implementation is clean and follows good practices. Here's my detailed analysis:

✅ Code Quality & Best Practices

Strong Points:

  • Clean separation of concerns: CSS styling in .file-size class (zip-wheel-explorer.html:90-95) is well-defined with appropriate visual hierarchy
  • Consistent naming: formatFileSize() function (zip-wheel-explorer.html:392-396) follows established naming conventions
  • DOM structure: The change from text-only to structured DOM elements (zip-wheel-explorer.html:367-373) improves maintainability
  • Good error handling: Existing error handling patterns are maintained

Minor Improvements:

  • Consider adding JSDoc comments for the new formatFileSize() function to document its behavior
  • The CSS could benefit from a CSS custom property for the muted color (#999) used in multiple places

✅ Implementation Analysis

File Size Calculation (zip-wheel-explorer.html:403-404):

  • Correctly uses uint8array.byteLength to get accurate byte counts
  • Properly handles the asynchronous ZIP extraction process

Display Logic (zip-wheel-explorer.html:392-396):

  • Uses standard binary thresholds (1024 bytes = 1 KB, 1048576 bytes = 1 MB)
  • Appropriate decimal precision (.toFixed(1)) for larger units
  • Falls back to byte display for small files

CSS Styling (zip-wheel-explorer.html:90-95):

  • white-space: nowrap prevents size text wrapping issues
  • Appropriate visual hierarchy with smaller font and muted color
  • Good spacing with margin-left: 0.5em

✅ Performance Considerations

  • Minimal overhead: File size calculation happens during existing ZIP parsing, no additional iterations
  • Efficient DOM updates: Uses appendChild() instead of string concatenation for better performance
  • No memory leaks: Properly handles the additional size property without retaining extra references

✅ Security Assessment

  • No new attack vectors: The implementation doesn't introduce any new security concerns
  • XSS protection: File sizes are numeric values, not user-controlled strings, so no additional escaping needed
  • Existing security: Maintains the existing escapeHtml() usage for filenames

⚠️ Minor Issues & Suggestions

  1. Accessibility: Consider adding aria-label to the file size spans for screen readers:

    <span class="file-size">${formatFileSize(file.size)}</span>
  2. Internationalization: The size formatting uses English units. Consider locale-aware formatting if i18n is planned.

  3. Edge Cases: Very large files (GB+) aren't handled, but this is probably acceptable for the target use case of Python packages.

❌ Test Coverage

Missing Tests:

  • No automated tests for formatFileSize() function
  • No tests for file size extraction accuracy
  • No visual regression tests for the new layout

Recommended Test Cases:

// Unit tests for formatFileSize()
assert.equal(formatFileSize(500), "500 bytes");
assert.equal(formatFileSize(1024), "1.0 KB");
assert.equal(formatFileSize(1048576), "1.0 MB");
assert.equal(formatFileSize(1536), "1.5 KB");

📋 Overall Assessment

This is a well-implemented feature that enhances user experience without compromising performance or security. The code follows existing patterns and maintains consistency with the rest of the application.

Recommendation: ✅ Approve with minor suggestions

The implementation is production-ready. The suggested improvements are enhancements rather than blocking issues.


@simonw
Copy link
Owner Author

simonw commented Mar 20, 2026

image

@simonw simonw merged commit 207d295 into main Mar 20, 2026
2 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants