Skip to content

Add toggle to hide and show overlay panel#183

Merged
simonw merged 1 commit intomainfrom
claude/overlay-panel-toggle-YftIB
Dec 25, 2025
Merged

Add toggle to hide and show overlay panel#183
simonw merged 1 commit intomainfrom
claude/overlay-panel-toggle-YftIB

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Dec 25, 2025

Modify pge-outages-hmb to add a toggle for hiding and showing the content of the overlay panel - it defaults to visible but you can click to hide it which reduces it to just the PG&E Outage Map header and an expand button. Save the hide/expand state in a localStorage key

Add a toggle button (-/+) to hide/show the info panel content while keeping the header visible. State is persisted in localStorage using the key 'pge-outages-hmb-panel-collapsed'. Panel defaults to expanded.

https://gistpreview.github.io/?393a8596cbb1e26065fa8f2aa866e245/index.html

Add a toggle button (-/+) to hide/show the info panel content while
keeping the header visible. State is persisted in localStorage using
the key 'pge-outages-hmb-panel-collapsed'. Panel defaults to expanded.
@claude
Copy link
Contributor

claude bot commented Dec 25, 2025

Claude finished @simonw's task —— View job


Code Review for Toggle Feature

Reviewing the pull request that adds a toggle to hide/show the overlay panel content with localStorage persistence.

Todo List:

  • Read repository guidelines (CLAUDE.md) - No repo-specific guidelines found
  • Read and analyze the modified pge-outages-hmb.html file
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Identify security concerns
  • Evaluate test coverage needs
  • Provide comprehensive feedback

Review Feedback

Code Quality & Best Practices

Excellent implementation overall! The toggle functionality is well-structured and follows good practices:

  • Clean separation of concerns: CSS animations, JavaScript state management, and DOM manipulation are properly organized
  • Semantic HTML: Good use of semantic elements and accessibility attributes (title attribute on button)
  • CSS transitions: Smooth animations with reasonable durations (0.3s ease-out, 0.2s ease-out)
  • Consistent naming: Clear, descriptive variable and function names

Implementation Strengths

  1. Proper state persistence: Uses localStorage with a meaningful key pge-outages-hmb-panel-collapsed
  2. User feedback: Button text and tooltip change appropriately (-/+ and corresponding titles)
  3. Smooth UX: CSS transitions provide polished user experience
  4. Initialization logic: Correctly restores panel state on page load

⚠️ Minor Issues & Suggestions

1. Accessibility Enhancement (pge-outages-hmb.html:164)

<button class="toggle-btn" id="toggle-btn" onclick="togglePanel()">-</button>

Consider adding aria-expanded for better screen reader support:

<button class="toggle-btn" id="toggle-btn" onclick="togglePanel()" 
        aria-expanded="true">-</button>

2. Error Handling (pge-outages-hmb.html:224)

const collapsed = localStorage.getItem(PANEL_STORAGE_KEY) === 'true';

Consider adding try-catch for localStorage access (can fail in private browsing):

function initPanelState() {
    try {
        const collapsed = localStorage.getItem(PANEL_STORAGE_KEY) === 'true';
        // ... rest of logic
    } catch (e) {
        console.warn('localStorage not available, using default state');
    }
}

3. CSS Specificity (pge-outages-hmb.html:68)

.info-panel-content.collapsed {
    max-height: 0 !important;

The !important could be avoided with more specific selectors, though it works fine here.

Performance Considerations

  • Efficient DOM operations: Minimal DOM queries, elements cached appropriately
  • CSS animations: Hardware-accelerated properties used (opacity, max-height)
  • No performance impact: Toggle functionality adds negligible overhead

Security Assessment

  • Safe localStorage usage: No sensitive data stored, only UI state
  • XSS prevention: No dynamic HTML injection in toggle functionality
  • No external dependencies: Toggle feature is self-contained

Test Coverage

While there are no formal tests, the implementation is straightforward enough that manual testing should suffice. The functionality could be tested by:

  • Verifying panel collapses/expands on button click
  • Confirming localStorage persistence across page reloads
  • Checking accessibility with screen readers

🎯 Overall Assessment

Excellent work! This is a clean, well-implemented feature that:

  • ✅ Meets all requirements in the PR description
  • ✅ Follows consistent code style with the existing codebase
  • ✅ Provides good user experience with smooth animations
  • ✅ Properly persists state across sessions
  • ✅ Has minimal security or performance concerns

The implementation is production-ready. The suggested improvements are minor enhancements for accessibility and error handling rather than critical issues.

Recommendation: APPROVE 🚀


@simonw simonw merged commit e16c9e2 into main Dec 25, 2025
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