-
Notifications
You must be signed in to change notification settings - Fork 5
fix: prevent Safari iOS 16 crash in charter flow #661
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughBrowser compatibility and transpilation settings are configured for Safari 16+ support. Next.js environment types are updated. The MaintenanceBanner component is corrected to properly fetch and display maintenance status data on mount. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
platforms/group-charter-manager/.browserslistrc (1)
1-8: Minor comment inconsistency with actual targets.The comment mentions "iOS 16+ Safari support" but the configuration targets iOS >= 15 and Safari >= 15. While this includes iOS 16, the comment could be more accurate to reflect the broader support range being configured.
📝 Suggested clarification
# Browsers that we support -# iOS 16+ Safari support +# iOS 15+ and Safari 15+ support (includes iOS 16 fix)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
platforms/group-charter-manager/.browserslistrcplatforms/group-charter-manager/next-env.d.tsplatforms/group-charter-manager/next.config.tsplatforms/group-charter-manager/src/components/MaintenanceBanner.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
platforms/group-charter-manager/next-env.d.ts (1)
3-3: LGTM! Standard Next.js typed routes feature.This reference enables type-safe routing in Next.js 15+, which is appropriate for your Next.js 15.5.9 installation.
platforms/group-charter-manager/next.config.ts (1)
8-12: Verify the transpilation fixes Safari 16 RegExp errors on a Safari iOS 16 device or simulator.The approach of using
transpilePackagesis correct for addressing Safari 16 compatibility issues. Bothmarked(v16.1.1) and@milkdown/preset-gfm(v7.15.2) are used in the codebase and will be transpiled to avoid unsupported RegExp syntax. The configuration is properly set up in next.config.ts.Since the PR description notes that "changes have not been tested yet," manual verification on an actual Safari iOS 16 device or simulator is required to confirm:
- The "Invalid regular expression: invalid group specifier name" error no longer occurs
- The Charter flow opens and functions correctly
- No ChunkLoadError is thrown
platforms/group-charter-manager/src/components/MaintenanceBanner.tsx (1)
20-24: [Rewritten comment]
[Classification tag]
Description of change
Fixes a UI crash on Safari iOS 16 when opening the Charter.
This prevents JS execution from failing and keeps the Charter flow usable on iOS 16.
Issue Number
Closes #636
Type of change
How the change has been tested
Changes not tested yet
Change checklist
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.