Skip to content

fix: Enable button clicks on mobile coming soon page#15

Closed
sethwebster wants to merge 6 commits into
mainfrom
wt-fix-buttons-mobile
Closed

fix: Enable button clicks on mobile coming soon page#15
sethwebster wants to merge 6 commits into
mainfrom
wt-fix-buttons-mobile

Conversation

@sethwebster

Copy link
Copy Markdown
Collaborator

Summary

  • Fixed button interaction issues on mobile devices on the coming soon page
  • 3D React logo canvas was blocking touch events
  • Added pointer-events-none to logo container
  • Improved button responsiveness with better mobile padding

Changes

  • Added pointer-events-none class to ReactLogo3D container div
  • Made buttons more mobile-friendly with responsive padding and text sizes

Test Plan

  • Test button clicks on mobile devices (iOS Safari, Android Chrome)
  • Verify 3D logo still animates correctly
  • Confirm GitHub sign-in button works on mobile
  • Verify "Request Access" button works on mobile

- Fix missing ThemeSegmentedControl component (use ThemeToggle instead)
- Fix button variant types in theme test pages
- Use Vercel VERCEL_URL for production email links
- Add 5-second polling to detect access approval in real-time
- Replace purples with amber/orange complementary colors

Theme system now works properly.
Email links use correct production URLs.
Users auto-redirect when approved (no refresh needed).
- Wrap app in ThemeProvider to fix useTheme error
- Theme toggle now works in header

Fixes runtime error: 'useTheme must be used within a ThemeProvider'
- Set defaultTheme='dark' in ThemeProvider
- Site is designed for dark mode, not light

Fixes bright white borders appearing on home page.
3D React logo canvas was capturing touch events, preventing button interactions on mobile. Added pointer-events-none to logo container and improved button responsiveness.
@vercel

vercel Bot commented Oct 23, 2025

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
react-foundation Error Error Oct 23, 2025 0:19am

@claude

claude Bot commented Oct 23, 2025

Copy link
Copy Markdown

Code Review - PR #15

Summary

This PR addresses mobile button interaction issues and introduces a comprehensive theming system. While the core fix is solid, there are several critical concerns.

Positive Aspects

  1. Mobile fix is correct - pointer-events-none properly prevents canvas from blocking touch
  2. Theming architecture is well-designed with clean separation of concerns
  3. Comprehensive documentation with excellent THEMING_GUIDE.md

Critical Issues

1. Major Version Bump: Next.js 15 to 16

  • Next.js 16 is a major version with breaking changes
  • NOT mentioned in PR title or description
  • Recommendation: Separate into its own PR with proper testing

2. Scope Creep

  • 90+ files modified (3,323 additions, 896 deletions)
  • PR says it fixes mobile buttons but includes massive theming refactor
  • Actual mobile fix is ~5 lines, remaining 3,300+ lines are theming work
  • Recommendation: Split into two PRs

3. Incomplete Implementation

  • Despite theming system, many hardcoded colors remain in coming-soon/page.tsx
  • Lines 74, 105, 111, 119, 127, 143, 147, 156 have hardcoded cyan/orange/amber
  • Recommendation: Fully migrate or document exceptions

Other Concerns

  • Test plan checkboxes all unchecked
  • Vercel build FAILED
  • Theme config duplication (theme.ts vs theme-config.ts)
  • Debug file committed (debug-theme.html)
  • No test coverage
  • Accessibility concerns with contrast ratios

Security

No security concerns identified

Performance

Minor concerns with theme switching and hydration. Consider using next-themes library.

Recommendations

CRITICAL:

  • Separate Next.js 16 upgrade into own PR
  • Fix Vercel build failures
  • Split into two PRs (mobile fix + theming)

HIGH:

  • Complete test plan

MEDIUM:

  • Consolidate theme configs
  • Fix or document hardcoded colors

Score: 6.5/10

  • Mobile Fix: 9/10
  • Theming System: 6/10
  • Testing: 2/10
  • Scope Management: 3/10
  • Security: 10/10

Verdict: Request Changes

The mobile fix is excellent but bundling it with massive theming refactor and Next.js 16 upgrade makes this too risky.

Suggested path:

  1. Create minimal PR with just pointer-events-none fix
  2. Merge quickly (blocking mobile users)
  3. Continue theming work in separate PR
  4. Separate PR for Next.js 16 upgrade

Claude Code Review - following CLAUDE.md guidelines

@sethwebster sethwebster deleted the wt-fix-buttons-mobile branch October 23, 2025 11:55
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.

1 participant