Improve login and signup error feedback for better user experience#369
Improve login and signup error feedback for better user experience#369anishghanwat wants to merge 39 commits into
Conversation
…s the application
… Timeline component
…d snapping feedback
…using CSS background instead
👷 Deploy request for appcut pending review.Visit the deploys page to approve it
|
|
@anishghanwat is attempting to deploy a commit to the OpenCut OSS Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThis update introduces a centralized, type-safe environment variable management system across the codebase using Changes
Sequence Diagram(s)Timeline Snapping Feature (High-Level)sequenceDiagram
participant User
participant Timeline
participant TimelineStore
participant useTimelineSnapping
participant SnapIndicator
User->>Timeline: Drag timeline element
Timeline->>TimelineStore: Get snappingEnabled, dragState
Timeline->>useTimelineSnapping: snapElementPosition(...)
useTimelineSnapping-->>Timeline: { snappedTime, snapPoint }
Timeline->>SnapIndicator: Show snap indicator (if snapPoint)
Timeline->>TimelineStore: Add element at snapped position (if dropped)
Centralized Environment Variable AccesssequenceDiagram
participant AppCode
participant keys() / env
participant @t3-oss/env
participant process.env
AppCode->>keys() / env: Import and call for config
keys() / env->>@t3-oss/env: Validate env vars with schema
@t3-oss/env->>process.env: Read raw env vars
@t3-oss/env-->>keys() / env: Return validated config
keys() / env-->>AppCode: Provide typed config values
Possibly related PRs
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
apps/web/src/hooks/auth/useSignUp.ts (1)
55-69: Remove or Disable Google Sign-Up to Match Server ConfigurationThe server no longer supports Google OAuth (packages/auth/src/server.ts has no “google” provider), yet the client hook still calls
signIn.social({ provider: "google" }). This mismatch will throw at runtime.Please update the client to align with server capabilities:
• File: apps/web/src/hooks/auth/useSignUp.ts
Lines: ~55–69
– Remove or guard thehandleGoogleSignUpcallback.
– Remove “Google” from any UI sign-up options and thesignIn.socialprovider list.If you intend to keep Google OAuth, re-add the “google” provider configuration in packages/auth/src/server.ts instead.
packages/auth/src/server.ts (1)
13-22: Action Needed: Sync Google OAuth Changes Between Server and FrontendThe server’s Google OAuth configuration has been removed, but the frontend still calls
handleGoogleSignUp, leading to a broken signup flow. Please choose one of the following:
Remove Google signup from the frontend
- apps/web/src/hooks/auth/useSignUp.ts
• Delete thehandleGoogleSignUpcallback and relatedisGoogleLoadingstate.- apps/web/src/app/(auth)/signup/page.tsx
• Remove the Google signup<Button>and any props tied tohandleGoogleSignUporisGoogleLoading.Restore Google OAuth on the server
- Re-add the Google provider settings in
packages/auth/src/server.tsto match the existing frontend implementation.
🧹 Nitpick comments (7)
apps/web/drizzle.config.ts (1)
6-10: Potential redundancy with dotenv loading.Since you're now using a centralized environment system that likely handles environment variable loading, the manual
dotenv.config()calls might be redundant or could potentially conflict with the centralized system.Consider verifying if the centralized env system already handles loading the appropriate environment files, and if so, remove the manual dotenv configuration:
-// Load the right env file based on environment -if (env.NODE_ENV === "production") { - dotenv.config({ path: ".env.production" }); -} else { - dotenv.config({ path: ".env.local" }); -}packages/db/drizzle.config.ts (1)
8-12: Potential redundancy with dotenv loading.Similar to the web app's drizzle config, the manual
dotenv.config()calls might be redundant since the centralized environment system likely handles environment file loading.Consider verifying if the centralized env system already handles loading the appropriate environment files, and if so, remove the manual dotenv configuration:
-// Load the right env file based on environment -if (NODE_ENV === "production") { - dotenv.config({ path: ".env.production" }); -} else { - dotenv.config({ path: ".env.local" }); -}apps/web/src/env.ts (1)
19-19: Consider adding client environment variables if needed.The client environment object is currently empty. If the web app requires any client-side environment variables in the future, they should be added here to maintain the centralized configuration pattern.
apps/web/src/hooks/auth/useLogin.ts (1)
63-63: Consider using the error helper for Google login consistency.For consistency with email login error handling, consider using the
getFriendlyLoginErrorhelper for Google login errors as well, or create a similar helper for social login errors.- setError("Failed to sign in with Google. Please try again."); + setError(getFriendlyLoginError("Failed to sign in with Google. Please try again."));apps/web/src/app/roadmap/page.tsx (2)
10-90: Well-structured roadmap data with minor content concerns.The roadmap data structure is well-typed and comprehensive. However, consider these improvements:
- Professional tone: Line 39 contains strong language ("fucking perfect") that might not be appropriate for all audiences.
- Content consistency: Ensure all descriptions maintain a professional tone while being engaging.
- "The heart of any video editor. Timeline zoom in/out, making clips longer/shorter, dragging elements around, selection, playhead scrubbing. **This part has to be fucking perfect** because it's what users interact with 99% of the time.", + "The heart of any video editor. Timeline zoom in/out, making clips longer/shorter, dragging elements around, selection, playhead scrubbing. **This part has to be absolutely perfect** because it's what users interact with 99% of the time.",
144-144: Consider dynamic date handling for maintenance.The hardcoded "last updated" date will require manual updates. Consider either:
- Making it dynamic based on file modification time
- Adding a TODO comment to remind developers to update it
- Moving it to a configuration constant
- What's coming next for OpenCut (last updated: July 14, 2025) + What's coming next for OpenCut (last updated: {new Date().toLocaleDateString('en-US', { year: 'numeric', month: 'long', day: 'numeric' })})apps/web/src/components/editor/media-panel/views/text.tsx (1)
6-27: Consider making textData immutable.The
textDataobject contains well-structured default values, but it's declared withletwhile being used as a constant. Consider usingconstinstead to prevent accidental mutations.-let textData: TextElement = { +const textData: TextElement = {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/web/public/open-graph/default.jpgis excluded by!**/*.jpgapps/web/public/open-graph/roadmap.jpgis excluded by!**/*.jpgbun.lockis excluded by!**/*.lock
📒 Files selected for processing (42)
.github/CONTRIBUTING.md(0 hunks)README.md(1 hunks)apps/web/.env.example(0 hunks)apps/web/drizzle.config.ts(1 hunks)apps/web/migrations/0002_cuddly_pretty_boy.sql(1 hunks)apps/web/migrations/meta/0002_snapshot.json(1 hunks)apps/web/migrations/meta/_journal.json(1 hunks)apps/web/package.json(3 hunks)apps/web/src/app/layout.tsx(0 hunks)apps/web/src/app/metadata.ts(1 hunks)apps/web/src/app/page.tsx(1 hunks)apps/web/src/app/roadmap/page.tsx(1 hunks)apps/web/src/components/development-debug.tsx(0 hunks)apps/web/src/components/editor/media-panel/views/media.tsx(4 hunks)apps/web/src/components/editor/media-panel/views/text.tsx(2 hunks)apps/web/src/components/editor/snap-indicator.tsx(1 hunks)apps/web/src/components/editor/timeline-element.tsx(2 hunks)apps/web/src/components/editor/timeline-playhead.tsx(3 hunks)apps/web/src/components/editor/timeline-track.tsx(6 hunks)apps/web/src/components/editor/timeline.tsx(11 hunks)apps/web/src/components/footer.tsx(1 hunks)apps/web/src/components/landing/hero.tsx(1 hunks)apps/web/src/components/ui/draggable-item.tsx(6 hunks)apps/web/src/components/ui/tooltip.tsx(1 hunks)apps/web/src/constants/timeline-constants.ts(1 hunks)apps/web/src/env.ts(1 hunks)apps/web/src/hooks/auth/useLogin.ts(2 hunks)apps/web/src/hooks/auth/useSignUp.ts(2 hunks)apps/web/src/hooks/use-timeline-snapping.ts(1 hunks)apps/web/src/lib/rate-limit.ts(1 hunks)apps/web/src/middleware.ts(2 hunks)apps/web/src/stores/timeline-store.ts(5 hunks)docker-compose.yaml(0 hunks)packages/auth/package.json(1 hunks)packages/auth/src/client.ts(1 hunks)packages/auth/src/index.ts(1 hunks)packages/auth/src/keys.ts(1 hunks)packages/auth/src/server.ts(2 hunks)packages/db/drizzle.config.ts(1 hunks)packages/db/package.json(2 hunks)packages/db/src/index.ts(1 hunks)packages/db/src/keys.ts(1 hunks)
💤 Files with no reviewable changes (5)
- apps/web/.env.example
- apps/web/src/app/layout.tsx
- .github/CONTRIBUTING.md
- docker-compose.yaml
- apps/web/src/components/development-debug.tsx
🧰 Additional context used
🧠 Learnings (12)
apps/web/src/components/ui/tooltip.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/constants/timeline-constants.ts (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/media-panel/views/text.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-playhead.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/media-panel/views/media.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-track.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/snap-indicator.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline-element.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/ui/draggable-item.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/hooks/use-timeline-snapping.ts (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/stores/timeline-store.ts (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
apps/web/src/components/editor/timeline.tsx (1)
Learnt from: simonorzel26
PR: OpenCut-app/OpenCut#324
File: apps/web/src/components/editor/snap-indicator.tsx:43-43
Timestamp: 2025-07-17T08:26:10.929Z
Learning: In the timeline refactor PR #324, the snap indicator component in apps/web/src/components/editor/snap-indicator.tsx requires the hard-coded `ml-48` class in addition to the calculated `leftPosition` for proper alignment. This is intentional and needed for the new timeline layout.
🧬 Code Graph Analysis (19)
apps/web/drizzle.config.ts (1)
apps/web/src/env.ts (1)
env(7-27)
packages/db/src/index.ts (1)
packages/db/src/keys.ts (1)
keys(4-19)
apps/web/src/middleware.ts (1)
apps/web/src/env.ts (1)
env(7-27)
packages/auth/src/client.ts (1)
packages/auth/src/keys.ts (1)
keys(4-16)
apps/web/src/app/page.tsx (1)
apps/web/src/components/landing/hero.tsx (1)
Hero(13-133)
apps/web/src/components/footer.tsx (1)
apps/web/src/lib/fetch-github-stars.ts (1)
getStars(1-29)
packages/db/src/keys.ts (1)
packages/auth/src/keys.ts (1)
keys(4-16)
apps/web/src/components/editor/media-panel/views/text.tsx (3)
apps/web/src/types/timeline.ts (1)
TextElement(23-38)apps/web/src/constants/timeline-constants.ts (1)
TIMELINE_CONSTANTS(72-79)apps/web/src/stores/timeline-store.ts (1)
useTimelineStore(189-1129)
apps/web/src/lib/rate-limit.ts (1)
apps/web/src/env.ts (1)
env(7-27)
packages/auth/src/server.ts (2)
packages/auth/src/keys.ts (1)
keys(4-16)packages/db/src/index.ts (1)
db(20-20)
packages/auth/src/keys.ts (1)
packages/db/src/keys.ts (1)
keys(4-19)
packages/db/drizzle.config.ts (2)
packages/auth/src/keys.ts (1)
keys(4-16)packages/db/src/keys.ts (1)
keys(4-19)
apps/web/src/env.ts (2)
packages/auth/src/server.ts (1)
auth(8-25)packages/db/src/index.ts (1)
db(20-20)
apps/web/src/components/editor/media-panel/views/media.tsx (1)
apps/web/src/stores/timeline-store.ts (1)
useTimelineStore(189-1129)
apps/web/src/components/editor/timeline-element.tsx (1)
apps/web/src/constants/timeline-constants.ts (1)
getTrackHeight(43-45)
apps/web/src/components/ui/draggable-item.tsx (1)
apps/web/src/stores/playback-store.ts (1)
usePlaybackStore(53-134)
apps/web/src/app/roadmap/page.tsx (4)
apps/web/src/components/header.tsx (1)
Header(12-70)apps/web/src/components/icons.tsx (1)
GithubIcon(24-39)apps/web/src/lib/utils.ts (1)
cn(6-8)apps/web/src/components/footer.tsx (1)
Footer(10-140)
apps/web/src/hooks/use-timeline-snapping.ts (2)
apps/web/src/types/timeline.ts (1)
TimelineTrack(81-88)apps/web/src/constants/timeline-constants.ts (1)
TIMELINE_CONSTANTS(72-79)
apps/web/src/stores/timeline-store.ts (4)
apps/web/src/types/timeline.ts (3)
TrackType(4-4)TextElement(23-38)DragData(79-79)apps/web/src/stores/media-store.ts (1)
MediaItem(8-26)apps/web/src/constants/timeline-constants.ts (1)
TIMELINE_CONSTANTS(72-79)apps/web/src/hooks/use-toast.ts (1)
toast(191-191)
🔇 Additional comments (60)
apps/web/src/components/landing/hero.tsx (1)
13-13: Inconsistency between PR objectives and actual changes.The PR objectives state this is about "improving login and signup error feedback," but the actual changes remove waitlist count functionality from the Hero component. The function signature change from accepting props to no props appears correct based on the AI summary, but these changes don't align with the stated goal of improving error messaging.
Could you clarify if this PR includes additional files that actually implement the login/signup error feedback improvements mentioned in the objectives?
Likely an incorrect or invalid review comment.
apps/web/src/app/page.tsx (1)
9-9: Implementation is consistent with Hero component changes.The removal of the
signupCountprop correctly aligns with the Hero component's updated signature. This simplifies the component rendering and removes the dynamic waitlist count functionality.However, this change still doesn't relate to the PR objectives about improving login and signup error feedback.
Likely an incorrect or invalid review comment.
README.md (1)
150-153: LGTM! Proper sponsor acknowledgment.The addition of the Vercel OSS Program badge appropriately acknowledges their open-source support and follows best practices with proper alt text for accessibility.
apps/web/src/app/metadata.ts (1)
6-7: Confirm Open Graph image availability and redirectsThe new
apps/web/public/open-graph/default.jpgfile is present.Before merging:
- Verify that requests to
/opengraph-image.jpgare redirected to/open-graph/default.jpgto prevent broken social previews.- Ensure the image dimensions remain 1200×630 for proper Open Graph rendering.
apps/web/migrations/0002_cuddly_pretty_boy.sql (1)
1-1: LGTM! Proper security default for email verification.Setting
email_verifiedto defaultfalsefollows security best practices by ensuring users start in an unverified state. This aligns well with the PR's goal of improving login/signup error feedback.Note: This migration only affects new user records. Existing users'
email_verifiedstatus will remain unchanged, which is typically the desired behavior.apps/web/migrations/meta/_journal.json (1)
18-25: LGTM! Proper migration tracking.The journal entry correctly tracks the new migration with sequential indexing and consistent formatting. The timestamp and tag properly correspond to the migration file.
packages/auth/src/index.ts (1)
7-8: Approve addition of the keys module export
- packages/auth/src/keys.ts exists and exports a
keys()function usingcreateEnv- packages/auth/package.json exports include
"./keys": "./src/keys.ts"This aligns with the existing export patterns and centralizes environment-variable management as intended.
apps/web/package.json (3)
24-25: LGTM! Environment management dependencies properly added.The addition of
@t3-oss/env-coreand@t3-oss/env-nextjsdependencies supports the centralized, type-safe environment variable management refactoring seen throughout the codebase.
41-41: Next.js minor version upgrade looks good.The upgrade from
^15.3.4to^15.4.1is a minor version bump that should be safe and may include bug fixes and improvements.
50-50: react-markdown addition supports new functionality.The addition of
react-markdownat version^10.1.0appears to support markdown rendering capabilities, likely for the roadmap page mentioned in the AI summary.packages/db/src/index.ts (3)
4-4: Good addition of centralized environment configuration.The import of the
keysfunction supports the type-safe environment variable management refactoring.
6-6: Excellent refactoring to use validated environment variables.Replacing direct
process.env.DATABASE_URLaccess with the validatedDATABASE_URLfromkeys()improves type safety and centralizes environment configuration. Thekeys()function frompackages/db/src/keys.tsproperly validates thatDATABASE_URLstarts withpostgres://orpostgresql://.
12-12: Database client initialization properly uses validated environment variable.Using the validated
DATABASE_URLfrom the centralized configuration maintains the same functionality while improving type safety.apps/web/src/middleware.ts (2)
3-3: Good addition of centralized environment configuration.The import of the
envobject supports the type-safe environment variable management refactoring.
13-13: Excellent refactoring to use validated NODE_ENV.Replacing
process.env.NODE_ENVwithenv.NODE_ENVimproves type safety while maintaining the same conditional logic. Theenvobject fromapps/web/src/env.tsprovides validated access toNODE_ENVwith proper typing.apps/web/src/lib/rate-limit.ts (2)
4-4: Good addition of centralized environment configuration.The import of the
envobject supports the type-safe environment variable management refactoring.
7-8: Excellent refactoring to use validated Redis configuration.Replacing direct
process.envaccess withenv.UPSTASH_REDIS_REST_URLandenv.UPSTASH_REDIS_REST_TOKENimproves type safety while maintaining the same Redis client configuration. Theenvobject fromapps/web/src/env.tsvalidates these as proper string/URL types.packages/db/package.json (3)
11-11: Good addition of keys export.The new export path
"./keys": "./src/keys.ts"properly exposes the centralized environment configuration functionality to other packages.
20-20: Appropriate dependency addition for environment management.The
@t3-oss/env-nextjsdependency at version^0.13.8is required for thecreateEnvfunctionality used in the newkeys.tsmodule.
23-23: Zod dependency properly supports environment validation.The
zoddependency at version^4.0.5is required for the schema validation in the environment configuration setup.apps/web/src/components/footer.tsx (3)
6-6: LGTM: Discord icon import added correctly.The
RiDiscordFillimport follows the existing pattern and is properly used in the component.
62-69: LGTM: Discord social link implementation.The Discord link follows the same structure and styling as the existing GitHub and Twitter links, maintaining consistency in the footer's social media section.
78-84: LGTM: Roadmap link added to Resources section.The roadmap link is properly integrated into the Resources section with consistent styling and structure.
apps/web/drizzle.config.ts (2)
3-3: LGTM: Centralized environment import.The import of the centralized
envobject improves type safety and consistency across the codebase.
16-16: LGTM: Using validated environment variables.The replacement of direct
process.envaccess with the validatedenvobject provides better type safety and validation.Also applies to: 19-19
packages/auth/src/client.ts (3)
2-2: LGTM: Centralized keys import.The import of the
keysfunction aligns with the codebase's move towards centralized environment variable management.
4-4: LGTM: Type-safe environment variable access.Destructuring
NEXT_PUBLIC_BETTER_AUTH_URLfrom the validatedkeys()function provides better type safety and URL validation compared to directprocess.envaccess.
7-7: LGTM: Using validated environment variable.The
baseURLnow uses the validated environment variable, which ensures it's a proper URL string as enforced by the zod schema in the keys function.packages/auth/package.json (1)
11-11: LGTM: Keys module export added.The new export path makes the keys module available as part of the package's public API, supporting the centralized environment variable management.
packages/db/drizzle.config.ts (3)
3-3: LGTM: Centralized keys import.The import of the
keysfunction provides validated environment variables with proper schema validation.
5-5: LGTM: Type-safe environment variable destructuring.Destructuring from the validated
keys()function ensuresNODE_ENVis a proper enum value andDATABASE_URLfollows the correct PostgreSQL URL format.
21-21: LGTM: Using validated environment variables.The replacement of direct
process.envaccess with validated variables from thekeys()function provides better type safety and ensures proper PostgreSQL URL format validation.Also applies to: 24-24
packages/auth/src/keys.ts (1)
1-16: LGTM! Excellent centralized environment variable management.This implementation follows best practices for type-safe environment variable handling using
@t3-oss/env-nextjsandzod. The separation of server and client variables is appropriate for Next.js, and the URL validation forNEXT_PUBLIC_BETTER_AUTH_URLadds an extra layer of safety.The pattern is consistent with the similar implementation in
packages/db/src/keys.ts, promoting a unified approach across the codebase.packages/db/src/keys.ts (1)
1-19: LGTM! Robust database environment variable validation.The implementation provides excellent validation for database-related environment variables:
NODE_ENVenum validation with sensible default prevents invalid environment modesDATABASE_URLvalidation supports bothpostgres://andpostgresql://prefixes, accommodating common variations- Server-only configuration is appropriate for database credentials
The pattern aligns well with the auth package's approach, creating consistency across the codebase.
apps/web/src/hooks/auth/useSignUp.ts (1)
5-25: Excellent user-friendly error messaging implementation.The
getFriendlySignUpErrorhelper function effectively addresses the PR objective by providing clear, user-friendly error messages for common signup issues. The error mapping is comprehensive and covers:
- Duplicate account scenarios
- Email validation issues
- Password strength requirements
- Rate limiting
- Network connectivity problems
The fallback to the original message ensures no errors are lost while still improving the user experience for common cases.
apps/web/src/env.ts (1)
1-27: Excellent centralized environment configuration using composition.This implementation elegantly combines environment schemas from multiple sources using the
extendspattern:
- Vercel preset for deployment-specific variables
- Auth and DB keys for package-specific environment variables
- Additional app-specific variables (Redis, analysis tools)
The composition approach promotes maintainability and ensures consistency across the application stack.
packages/auth/src/server.ts (1)
4-6: LGTM! Improved environment variable management.The refactor to use the centralized
keys()function provides type-safe access to environment variables and replaces directprocess.envusage. This aligns with the broader effort to standardize environment variable handling across the codebase.apps/web/src/hooks/auth/useLogin.ts (2)
5-25: Excellent error mapping implementation!The
getFriendlyLoginErrorhelper function provides comprehensive coverage of common authentication error scenarios with user-friendly messaging. The case-insensitive matching and fallback to the original message ensure robust error handling.
45-45: Good integration of the error helper!The helper function is properly integrated for email login error handling.
apps/web/migrations/meta/0002_snapshot.json (1)
1-359: Database schema snapshot looks appropriate.This is a generated migration snapshot file that properly defines the database schema with appropriate table structures, foreign key relationships, and row-level security enabled. The schema appears consistent with a typical authentication system.
apps/web/src/app/roadmap/page.tsx (2)
119-247: Excellent component implementation!The
RoadmapPagecomponent is well-structured with:
- Proper TypeScript usage and React patterns
- Responsive design with Tailwind CSS
- Good accessibility with semantic HTML
- Proper integration with existing design system components
- Well-configured ReactMarkdown with custom link and text styling
- Nice visual touches with background gradients
The component follows the app's established patterns and provides a great user experience.
92-117: Metadata configuration and Open Graph image verification complete.The referenced image exists at
apps/web/public/open-graph/roadmap.jpg, which aligns with the URL/open-graph/roadmap.jpgin your metadata. No further changes are required.apps/web/src/constants/timeline-constants.ts (1)
77-77: LGTM! Consistent addition of image duration constant.The new
DEFAULT_IMAGE_DURATIONconstant follows the same pattern asDEFAULT_TEXT_DURATIONand provides a sensible default value.apps/web/src/components/editor/media-panel/views/text.tsx (1)
46-48: LGTM! Clean integration with timeline store.The
onAddToTimelinecallback properly integrates with the timeline store'saddTextAtTimemethod, allowing seamless addition of text elements to the timeline.apps/web/src/components/editor/media-panel/views/media.tsx (2)
211-211: LGTM! Improved layout and user experience.Removing the "Add media" button from the top control bar and repositioning it below the "No media in project" message creates a more intuitive user flow.
292-296: LGTM! Clean timeline integration.The
onAddToTimelinecallback properly integrates media items with the timeline store, enabling direct addition of media from the panel to the timeline at the current time.apps/web/src/components/editor/snap-indicator.tsx (1)
16-54: LGTM! Well-implemented snap indicator component.The component provides clean visual feedback for snapping functionality with:
- Proper position calculations accounting for zoom and track labels
- Appropriate styling with opacity and z-index
- Disabled pointer events to avoid interaction interference
- Good conditional rendering logic
The implementation follows React best practices and integrates well with the timeline system.
apps/web/src/components/editor/timeline-track.tsx (1)
1-1000: Inconsistency between PR objectives and actual changes.The PR description mentions "Improve login and signup error feedback for better user experience," but these changes implement timeline snapping functionality. Please verify that the correct files are being reviewed for this PR.
Technical implementation looks solid.
The snapping integration is well-implemented with:
- Proper conditional logic between snapping and frame-based positioning
- Clean snap point notification management
- Good separation of concerns with the useTimelineSnapping hook
- Appropriate snap point clearing on drag end
Likely an incorrect or invalid review comment.
apps/web/src/components/editor/timeline-playhead.tsx (1)
1-117: PR content mismatch: Timeline editor changes instead of login/signup error feedbackThe PR objectives state this is about "Improve login and signup error feedback for better user experience", but this file contains timeline editor functionality for visual snapping feedback. This appears to be content from a different PR.
Likely an incorrect or invalid review comment.
apps/web/src/components/editor/timeline-element.tsx (1)
1-471: PR content mismatch: Timeline element rendering instead of login/signup improvementsThis file implements tiled background rendering for timeline media elements, which is unrelated to the PR's stated objective of improving login and signup error feedback.
Likely an incorrect or invalid review comment.
apps/web/src/components/ui/draggable-item.tsx (1)
1-167: PR content mismatch: Draggable timeline items instead of authentication error handlingThis file adds timeline editing features for draggable media items, which doesn't align with the PR objective of improving login and signup error feedback.
Likely an incorrect or invalid review comment.
apps/web/src/hooks/use-timeline-snapping.ts (1)
1-179: PR content mismatch: Timeline snapping logic instead of authentication UX improvementsThis new hook implements timeline element snapping functionality, which is completely unrelated to the PR's stated goal of enhancing login and signup error messages for better user experience.
Likely an incorrect or invalid review comment.
apps/web/src/components/editor/timeline.tsx (1)
1-1076: PR content mismatch: Timeline editor implementation instead of authentication error feedbackThis comprehensive timeline component adds snapping functionality, drag-and-drop improvements, and UI enhancements for video editing. However, the PR is supposed to be about "Improve login and signup error feedback for better user experience" according to the objectives.
None of the code changes in this file relate to:
- Login/signup pages
- Error messaging
- Authentication feedback
- User experience during failed login/signup attempts
This appears to be content from an entirely different pull request.
Please verify that the correct files are being reviewed for PR #369. The PR objectives mention authentication improvements, but all provided files contain timeline editor functionality.
Likely an incorrect or invalid review comment.
apps/web/src/stores/timeline-store.ts (7)
52-56: Well-structured interface additions for snapping and element management.The new properties and methods follow good design patterns:
- Clear boolean flag for snapping state with a toggle method
- Consistent method signatures that return success/failure status
- Logical separation between time-specific and track-specific element addition methods
Also applies to: 176-186
230-231: Correct implementation of snapping toggle functionality.The snapping feature is properly implemented with sensible defaults (enabled by default) and uses the correct Zustand pattern for state updates.
Also applies to: 980-983
985-1008: Robust overlap detection implementation.The method correctly handles all overlap scenarios and edge cases:
- Proper calculation of effective element boundaries considering trim values
- Comprehensive overlap checks covering all possible time range intersections
- Defensive programming with track existence validation
1010-1022: Smart track management strategy.The implementation correctly handles different track types:
- Always creating new text tracks enables multiple text overlays (good for video editing UX)
- Reusing existing media/audio tracks follows standard video editing patterns
1024-1048: Well-implemented media addition with overlap protection.The method properly handles media placement:
- Correct track type selection based on media type
- User-friendly error feedback via toast notifications
- Proper use of constants for default durations
- Complete element initialization with all required properties
1050-1075: Verify if text overlap checking is intentionally omitted.The implementation provides good defaults for text properties, but unlike
addMediaAtTime, it doesn't check for element overlap. This might be intentional (allowing text overlays), but please confirm this design decision.If overlap checking should be added, consider implementing it similar to the media method. Otherwise, consider adding a comment explaining why text elements can overlap.
1077-1091: Clean implementation for adding media at track start.The method correctly leverages existing functionality and places elements at time 0, which is appropriate for new track creation or reuse scenarios.
| sideOffset={sideOffset} | ||
| className={cn( | ||
| "z-50 overflow-hidden rounded-md bg-primary px-3 py-1.5 text-xs text-primary-foreground animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", | ||
| "z-50 overflow-hidden rounded-md bg-foreground px-3 py-1.5 text-xs text-background animate-in fade-in-0 zoom-in-95 data-[state=closed]:animate-out data-[state=closed]:fade-out-0 data-[state=closed]:zoom-out-95 data-[side=bottom]:slide-in-from-top-2 data-[side=left]:slide-in-from-right-2 data-[side=right]:slide-in-from-left-2 data-[side=top]:slide-in-from-bottom-2", |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify bg-foreground / text-background tokens exist and meet WCAG contrast
The tooltip background / text colours were inverted from the typical bg-primary text-primary-foreground pair to bg-foreground text-background.
That only works if both utility classes are present in your Tailwind config and the resulting colour combination passes the required contrast ratio in light- and dark-mode themes.
If either token is missing, the tooltip will render with default transparent/inherit values and become invisible or unreadable.
Run the script below to confirm the colour tokens are defined:
🏁 Script executed:
#!/usr/bin/env bash
# Verify that `foreground` and `background` colour tokens exist in Tailwind config
set -e
fd tailwind.config.js | while read -r cfg; do
echo "⮞ Checking $cfg"
if ! rg -q '"foreground":' "$cfg"; then
echo " ❌ missing 'foreground' token"
fi
if ! rg -q '"background":' "$cfg"; then
echo " ❌ missing 'background' token"
fi
doneLength of output: 46
Let’s search for all Tailwind config variants and explicitly check for both tokens:
🏁 Script executed:
#!/usr/bin/env bash
# Locate all Tailwind config files and verify 'foreground' and 'background' tokens
set -e
configs=$(fd --extension js --extension ts --extension cjs --extension mjs '^tailwind.config\.' -t f)
if [ -z "$configs" ]; then
echo "⚠️ No Tailwind config file found"
exit 0
fi
for cfg in $configs; do
echo "⮞ Checking $cfg"
if rg -q '"foreground":' "$cfg"; then
echo " ✅ 'foreground' token found"
else
echo " ❌ missing 'foreground' token"
fi
if rg -q '"background":' "$cfg"; then
echo " ✅ 'background' token found"
else
echo " ❌ missing 'background' token"
fi
doneLength of output: 548
Define missing foreground and background color tokens in your Tailwind config
The classes bg-foreground and text-background are currently undefined in apps/web/tailwind.config.ts, so the tooltip will render with no background or inherited text color. You need to add both tokens (and verify their WCAG contrast in light/dark modes):
• File to update:
• apps/web/tailwind.config.ts – under theme.extend.colors
• Example snippet (adjust values to match your design tokens):
// tailwind.config.ts
export default {
// …
theme: {
extend: {
colors: {
foreground: {
DEFAULT: 'hsl(var(--color-foreground) / <alpha-value>)',
},
background: {
DEFAULT: 'hsl(var(--color-background) / <alpha-value>)',
},
},
},
},
// …
}• After adding, confirm in both light and dark themes that bg-foreground/text-background meets the required contrast ratios.
🤖 Prompt for AI Agents
In apps/web/src/components/ui/tooltip.tsx at line 23, the Tailwind CSS classes
bg-foreground and text-background are used but not defined in the Tailwind
configuration. To fix this, open apps/web/tailwind.config.ts and add foreground
and background color tokens under theme.extend.colors with appropriate HSL
values referencing CSS variables, ensuring they support alpha transparency.
After adding these tokens, verify that the tooltip's background and text colors
meet WCAG contrast requirements in both light and dark modes.
8 participants
Description
This PR improves error feedback on both the login and signup pages. Users now see clear, user-friendly messages for common issues such as invalid credentials, missing accounts, unverified emails, weak passwords, and network errors. This enhances the user experience and accessibility.
Fixes: No specific issue, but addresses user confusion on failed login and signup attempts.
Type of change
How Has This Been Tested?
Test Configuration:
Screenshots (if applicable)
Checklist:
Additional context
This PR improves the user experience for failed logins and signups, helping users understand what went wrong and reducing confusion and support requests.
Summary by CodeRabbit
New Features
Enhancements
Bug Fixes
Chores
Database
Documentation
Refactor