Skip to content

Replace table with mobile-friendly cards and progress bars in github-ratelimit#101

Merged
simonw merged 1 commit intomainfrom
claude/mobile-ratelimit-progress-011CUoh8bmGQ8i6ViE3z4ex4
Nov 4, 2025
Merged

Replace table with mobile-friendly cards and progress bars in github-ratelimit#101
simonw merged 1 commit intomainfrom
claude/mobile-ratelimit-progress-011CUoh8bmGQ8i6ViE3z4ex4

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Nov 4, 2025

  • Replaced table layout with responsive card-based design
  • Added visual progress bars to show usage for each rate limit
  • Progress bars color-coded: green (normal), yellow (warning), red (critical)
  • Each card displays resource name, remaining count, progress bar, and reset time
  • Improved mobile responsiveness with stacked stats on small screens

Claude Code for web prompt:

Instead of a table on github-ratelimit do something more mobile friendly, and include a progress bar for each limit visualizing how much is used

…ratelimit

- Replaced table layout with responsive card-based design
- Added visual progress bars to show usage for each rate limit
- Progress bars color-coded: green (normal), yellow (warning), red (critical)
- Each card displays resource name, remaining count, progress bar, and reset time
- Improved mobile responsiveness with stacked stats on small screens
@claude
Copy link
Contributor

claude bot commented Nov 4, 2025

Claude finished @simonw's task —— View job


Code Review for Mobile-Friendly GitHub Rate Limit Display

Todo List:

  • Read and analyze the modified github-ratelimit.html file
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Evaluate performance considerations
  • Assess security concerns
  • Review test coverage implications
  • Provide comprehensive feedback

Code Review Feedback

Overall, this is a well-executed transformation from a table-based layout to a modern, mobile-responsive card interface. The code demonstrates good HTML/CSS practices and thoughtful UX design.

Strengths

Design & UX:

  • Excellent mobile responsiveness with proper viewport meta tag and media queries
  • Intuitive color-coded progress bars (green/yellow/red) for quick status assessment
  • Clean card-based layout that's much more readable than traditional tables
  • Smooth hover effects and transitions enhance user experience

Code Quality:

  • Well-structured CSS with logical organization and consistent naming conventions
  • Proper use of CSS custom properties for colors following GitHub's design system
  • Good semantic HTML structure with appropriate ARIA considerations
  • Clean separation of concerns between CSS and JavaScript

Functionality:

  • Robust error handling for authentication failures (lines 428-434)
  • Proper cleanup of disabled states and loading indicators
  • Smart time formatting with hours/minutes display for longer durations

⚠️ Areas for Improvement

Accessibility:

  • Missing ARIA labels: Progress bars should have role="progressbar" and aria-valuenow/aria-valuemax attributes for screen readers
  • Color-only information: The warning/critical states rely solely on color. Consider adding icons or text indicators
  • Focus management: No visible focus indicators for keyboard navigation

Performance:

  • DOM manipulation: Creating HTML via innerHTML (lines 396-420) is less secure than using createElement and textContent
  • Memory leaks: The setInterval in startAuthPoll() (line 292) could potentially leak if the page is navigated away

Security:

  • XSS risk: Using innerHTML with user data could be problematic if resource names were ever user-controlled (low risk in this case)
  • Token storage: Storing tokens in localStorage exposes them to XSS attacks. Consider using httpOnly cookies or sessionStorage

Code Organization:

  • Magic numbers: Hardcoded thresholds (10%, 25%) at lines 329-332 should be constants
  • Long function: The checkRateLimit() function (lines 338-439) is quite long and could be broken into smaller functions

🔧 Specific Recommendations

<div class="progress-bar ${remainingClass}" 
     role="progressbar" 
     aria-valuenow="${usedPercentage}" 
     aria-valuemax="100"
    
     style="width: ${usedPercentage}%">
/* Add focus indicators */
.rate-limit-card:focus-within {
  outline: 2px solid #0969da;
  outline-offset: 2px;
}
// Extract constants
const THRESHOLDS = {
  CRITICAL: 10,
  WARNING: 25
};

// Use safer DOM manipulation
const progressText = document.createElement('span');
progressText.className = 'progress-text';
progressText.textContent = `${used.toLocaleString()} / ${limit.toLocaleString()} used`;
progressBar.appendChild(progressText);

📊 Performance Considerations

  • The current implementation is lightweight and should perform well
  • Consider adding a loading skeleton for better perceived performance
  • The polling mechanism (1-second intervals) is reasonable for the auth flow

🔒 Security Assessment

  • Low risk overall, but XSS prevention could be improved
  • Token handling follows reasonable practices for a client-side app
  • External URL (https://tools.simonwillison.net/github-auth) should be verified as trusted

🧪 Test Coverage

  • No automated tests present (expected for a standalone HTML file)
  • Consider adding basic JavaScript unit tests for utility functions like formatTimeRemaining and getRemainingClass
  • Manual testing should cover mobile viewports and various rate limit scenarios

Summary

This is a solid improvement that significantly enhances the mobile experience and visual appeal. The main areas to address are accessibility enhancements and minor security hardening. The code quality is good and follows modern web development practices.


@simonw simonw merged commit feeb720 into main Nov 4, 2025
4 checks passed
@simonw
Copy link
Owner Author

simonw commented Nov 4, 2025

image

@simonw simonw deleted the claude/mobile-ratelimit-progress-011CUoh8bmGQ8i6ViE3z4ex4 branch November 4, 2025 23:22
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