Skip to content

Switch to GitHub tab instantly on ?repo= instead of waiting for assets#235

Merged
simonw merged 1 commit intomainfrom
claude/fix-repo-tab-switching-GvyXG
Feb 18, 2026
Merged

Switch to GitHub tab instantly on ?repo= instead of waiting for assets#235
simonw merged 1 commit intomainfrom
claude/fix-repo-tab-switching-GvyXG

Conversation

@simonw
Copy link
Owner

@simonw simonw commented Feb 18, 2026

Look at most recent changes to sloccount.html - then make it so even you load the page with a ?repo= it instantly swaps to the correct tab rather than only switching after assets have loaded

The tab switch and repo input population now happen at script parse
time rather than inside initializeWebPerl(), so the user sees the
correct tab immediately on page load instead of watching the Paste
tab flash first while WASM assets load.

https://claude.ai/code/session_01GbmREbKFYgg48akhQoy5wg

The tab switch and repo input population now happen at script parse
time rather than inside initializeWebPerl(), so the user sees the
correct tab immediately on page load instead of watching the Paste
tab flash first while WASM assets load.

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

claude bot commented Feb 18, 2026

Claude finished @simonw's task —— View job


PR Review: Switch to GitHub tab instantly on ?repo=

Review Tasks:

  • Read and analyze the modified file (sloccount.html)
  • Evaluate code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Review security concerns
  • Evaluate test coverage needs

Review Feedback

Code Quality & Best Practices

  • Clean separation of concerns: The change properly separates immediate UI updates from async initialization logic
  • Good variable reuse: The repoParam variable is correctly scoped and reused between the immediate tab switching and later analysis
  • Consistent code style: The new code follows the existing patterns and indentation style
  • Clear comments: The added comment "Switch to GitHub tab instantly, before assets load" clearly explains the intent

Performance Considerations

  • Excellent UX improvement: Moving tab switching from initializeWebPerl() (lines 555-567) to script parse time (lines 1188-1201) eliminates the visible flash of the Paste tab while WASM assets load
  • Minimal performance impact: The URL parameter parsing happens at script parse time instead of after initialization, but this is negligible overhead
  • Maintains async flow: The actual repository analysis still happens after assets are loaded, preserving the existing async architecture

Bug Analysis

  • No regressions: The logic flow is preserved - tab switching happens immediately, analysis still waits for assets
  • Proper DOM manipulation: The tab switching code correctly handles the active class toggles on both tab buttons and content areas
  • Variable scoping: repoParam is properly declared in the outer scope and available to both code blocks

⚠️ Security Considerations

  • URL parameter handling: The code reads repo parameter from URL and displays it in the input field (repoInput.value = repoParam). While this is likely safe for this use case, the parameter value should be validated
  • Recommendation: Consider adding basic validation to ensure repoParam doesn't contain malicious content before setting it to the input field:
    if (repoParam && typeof repoParam === 'string' && repoParam.length < 1000) {
        repoInput.value = repoParam;
    }

⚠️ Test Coverage

  • Manual testing needed: This UI timing change should be manually tested to ensure:
    • Visiting /sloccount.html?repo=owner/repo shows GitHub tab immediately
    • The analysis still triggers correctly after assets load
    • No JavaScript errors occur during the transition
  • Edge cases to test:
    • Invalid repo parameter values
    • Very long repo parameter strings
    • Special characters in repo parameters

📋 Minor Suggestions

  1. Code organization: Consider extracting the tab switching logic into a helper function since it's duplicated:

    function switchToGitHubTab() {
        tabs.forEach(t => t.classList.remove('active'));
        document.querySelector('[data-tab="github"]').classList.add('active');
        pasteTab.classList.remove('active');
        githubTab.classList.add('active');
        zipTab.classList.remove('active');
    }
  2. Error handling: The new code assumes DOM elements exist at parse time. Add safety checks:

    const githubTabButton = document.querySelector('[data-tab="github"]');
    if (githubTabButton) {
        // perform tab switching
    }

🎯 Overall Assessment

This is a well-executed performance improvement that addresses a real UX issue. The change is minimal, focused, and maintains the existing functionality while providing immediate visual feedback to users. The code quality is good and follows existing patterns.

Risk Level: Low
Recommendation: ✅ Approve with consideration for the minor security and testing suggestions above.


@simonw simonw merged commit 4d3809c into main Feb 18, 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