-
-
Notifications
You must be signed in to change notification settings - Fork 852
Onboarding dashboard background #2346
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
Conversation
|
WalkthroughA new Estimated code review effort🎯 3 (Moderate) | ⏱️ ~18 minutes
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (2)
✅ Files skipped from review due to trivial changes (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Files:
{packages/core,apps/webapp}/**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
Files:
apps/webapp/**/*.{ts,tsx}📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
Files:
⏰ 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). (23)
🔇 Additional comments (2)
✨ Finishing Touches
🧪 Generate unit tests
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 (
|
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)
apps/webapp/app/components/BackgroundWrapper.tsx (1)
8-46
: Consider refactoring inline styles and addressing mobile viewport issues.The implementation uses extensive inline styles which could be moved to CSS classes for better maintainability. Also, using
100vh
can cause viewport issues on mobile browsers where the viewport height changes when the address bar appears/disappears.Consider these improvements:
+/* Add to CSS file */ +.bg-menu-top { + background-image: url('~/assets/images/blurred-dashboard-background-menu-top.jpg'); + background-size: 260px auto; + background-position: left top; + background-repeat: no-repeat; + height: 100dvh; /* Use dynamic viewport height */ +} - <div - className="absolute left-0 top-0 w-[260px] bg-contain bg-left-top bg-no-repeat" - style={{ - backgroundImage: `url(${blurredDashboardBackgroundMenuTop})`, - aspectRatio: "auto", - height: "100vh", - backgroundSize: "260px auto", - }} - /> + <div className="absolute left-0 top-0 w-[260px] bg-menu-top" />Also consider using
100dvh
(dynamic viewport height) instead of100vh
for better mobile compatibility.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
apps/webapp/app/assets/images/blurred-dashboard-background-menu-bottom.jpg
is excluded by!**/*.jpg
apps/webapp/app/assets/images/blurred-dashboard-background-menu-top.jpg
is excluded by!**/*.jpg
apps/webapp/app/assets/images/blurred-dashboard-background-table.jpg
is excluded by!**/*.jpg
📒 Files selected for processing (7)
apps/webapp/app/components/BackgroundWrapper.tsx
(1 hunks)apps/webapp/app/components/layout/AppLayout.tsx
(1 hunks)apps/webapp/app/components/primitives/Spinner.tsx
(1 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
(3 hunks)apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
(2 hunks)apps/webapp/app/routes/_app.orgs.new/route.tsx
(2 hunks)apps/webapp/app/routes/confirm-basic-details.tsx
(3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
**/*.{ts,tsx}
: Always prefer using isomorphic code like fetch, ReadableStream, etc. instead of Node.js specific code
For TypeScript, we usually use types over interfaces
Avoid enums
No default exports, use function declarations
Files:
apps/webapp/app/components/primitives/Spinner.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
apps/webapp/app/components/layout/AppLayout.tsx
apps/webapp/app/components/BackgroundWrapper.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
apps/webapp/app/routes/_app.orgs.new/route.tsx
apps/webapp/app/routes/confirm-basic-details.tsx
{packages/core,apps/webapp}/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.github/copilot-instructions.md)
We use zod a lot in packages/core and in the webapp
Files:
apps/webapp/app/components/primitives/Spinner.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
apps/webapp/app/components/layout/AppLayout.tsx
apps/webapp/app/components/BackgroundWrapper.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
apps/webapp/app/routes/_app.orgs.new/route.tsx
apps/webapp/app/routes/confirm-basic-details.tsx
apps/webapp/**/*.{ts,tsx}
📄 CodeRabbit Inference Engine (.cursor/rules/webapp.mdc)
apps/webapp/**/*.{ts,tsx}
: In the webapp, all environment variables must be accessed through theenv
export ofenv.server.ts
, instead of directly accessingprocess.env
.
When importing from@trigger.dev/core
in the webapp, never import from the root@trigger.dev/core
path; always use one of the subpath exports as defined in the package's package.json.
Files:
apps/webapp/app/components/primitives/Spinner.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
apps/webapp/app/components/layout/AppLayout.tsx
apps/webapp/app/components/BackgroundWrapper.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
apps/webapp/app/routes/_app.orgs.new/route.tsx
apps/webapp/app/routes/confirm-basic-details.tsx
🧠 Learnings (2)
📚 Learning: applies to apps/webapp/app/v3/presenters/**/*.server.ts : favor the use of 'presenters' to move comp...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .cursor/rules/webapp.mdc:0-0
Timestamp: 2025-07-18T17:49:47.180Z
Learning: Applies to apps/webapp/app/v3/presenters/**/*.server.ts : Favor the use of 'presenters' to move complex loader code into a class, as seen in `app/v3/presenters/**/*.server.ts`.
Applied to files:
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx
apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx
📚 Learning: applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : we use zod a lot in packages/core and in the ...
Learnt from: CR
PR: triggerdotdev/trigger.dev#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-18T17:49:24.468Z
Learning: Applies to {packages/core,apps/webapp}/**/*.{ts,tsx} : We use zod a lot in packages/core and in the webapp
Applied to files:
apps/webapp/app/routes/confirm-basic-details.tsx
🧬 Code Graph Analysis (2)
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx (4)
apps/webapp/app/components/layout/AppLayout.tsx (1)
AppContainer
(4-16)apps/webapp/app/components/BackgroundWrapper.tsx (1)
BackgroundWrapper
(6-48)apps/webapp/app/components/primitives/Headers.tsx (1)
Header1
(32-50)apps/webapp/app/routes/resources.orgs.$organizationSlug.select-plan.tsx (1)
PricingPlans
(213-244)
apps/webapp/app/components/layout/AppLayout.tsx (1)
apps/webapp/app/utils/cn.ts (1)
cn
(77-79)
⏰ 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). (23)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (8, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (4, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (7, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (5, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (3, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (1, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (8, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (7, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (6, 8)
- GitHub Check: units / internal / 🧪 Unit Tests: Internal (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (4, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (5, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (2, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (3, 8)
- GitHub Check: units / webapp / 🧪 Unit Tests: Webapp (1, 8)
- GitHub Check: units / packages / 🧪 Unit Tests: Packages (1, 1)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - pnpm)
- GitHub Check: e2e / 🧪 CLI v3 tests (windows-latest - npm)
- GitHub Check: e2e / 🧪 CLI v3 tests (ubuntu-latest - pnpm)
- GitHub Check: typecheck / typecheck
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (14)
apps/webapp/app/components/BackgroundWrapper.tsx (1)
6-48
: LGTM! Well-structured component with proper layering.The component correctly implements the layered background approach with proper z-index management to ensure content visibility. The fixed-width left menu backgrounds and full-width right background create the desired dashboard effect.
apps/webapp/app/routes/_app.orgs.$organizationSlug_.select-plan/route.tsx (1)
52-68
: LGTM! Consistent layout restructuring aligned with PR objectives.The nested
AppContainer
>BackgroundWrapper
structure follows the same pattern applied across other routes in this PR. The styling enhancements (rounded corners, border, shadow) improve visual presentation while maintaining all existing functionality.apps/webapp/app/components/layout/AppLayout.tsx (1)
4-15
: LGTM! Clean API enhancement with proper class merging.The addition of the optional
className
prop follows established patterns in the codebase and enables the styling flexibility needed for the new layout structure. The use of thecn
utility ensures proper Tailwind class merging.apps/webapp/app/components/primitives/Spinner.tsx (1)
69-70
: LGTM! Appropriate color update for dark theme compatibility.The change from black to white spinner colors ensures proper visibility on the dark backgrounds introduced by
BackgroundWrapper
. The rgba values follow the existing pattern used in other color options.apps/webapp/app/routes/_app.orgs.$organizationSlug_.projects.new/route.tsx (1)
141-196
: LGTM! Consistent implementation of new layout pattern.The restructuring follows the same
AppContainer
>BackgroundWrapper
pattern applied across other routes in this PR. The enhanced styling forMainCenteredContainer
and the button variant update align with the overall UI improvements while maintaining all existing functionality.apps/webapp/app/routes/_app.orgs.new/route.tsx (4)
10-11
: LGTM!The new imports for
BackgroundWrapper
and updatedAppContainer
are correctly structured and align with the UI enhancement objectives.
98-100
: LGTM!The new container hierarchy with
AppContainer
,BackgroundWrapper
, and styledMainCenteredContainer
properly implements the UI enhancement goals. The styling classes are appropriate and the nesting structure is logical.Also applies to: 182-184
118-164
: LGTM!The form reorganization logically groups the company size and problem description inputs within the managed cloud conditional block. This improves the user experience by showing related fields together while maintaining all existing form logic and validation.
166-180
: LGTM!The form buttons maintain all existing functionality including conditional rendering of the cancel button and proper loading state handling, while fitting well within the new UI structure.
apps/webapp/app/routes/confirm-basic-details.tsx (5)
11-11
: LGTM!The
BackgroundWrapper
import is correctly added and follows the same pattern as other updated routes.
117-117
: LGTM!The icon color update from
text-amber-300
totext-amber-400
provides better visual consistency and slightly improved contrast.
142-144
: LGTM!The container structure follows the same consistent pattern as other updated routes, providing a unified UI experience with proper styling and background enhancements.
Also applies to: 241-242
171-230
: LGTM!The form field reorganization improves user experience by presenting fields in a logical order. The conditional rendering logic for email confirmation and referral source is correctly maintained.
225-225
: LGTM!The updated placeholder text to include "LLM" reflects current market trends and provides users with relevant examples of referral sources.
* Adds a background image dashboard wrapper * Dashboard background image * Adds a background image to the welcome onboarding page * Background is constructed of 3 images * Adds the background to the create org page * Adds a background to the choose plan page * Change the default button spinner color to white * Adds the background to the create new project page * Updates the invite team member page * Adds background image to received invite page
Adds an image of the dashboard behind the onboarding page: