-
Notifications
You must be signed in to change notification settings - Fork 3
fix(frontend): fix cannot redirect into chat/project page after create project from home page #151
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
WalkthroughThe changes update several frontend components. In the API layer, file type detection now recognizes Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API
participant Docker
Client->>API: Request to build and run Docker
API->>Docker: Log start of build process, execute Docker run command
Docker-->>API: Return success or error (e.g., container name conflict)
alt Container conflict
API->>Docker: Remove existing container (with logging)
API->>Docker: Retry Docker run command
else No conflict
API->>Client: Return container details
end
sequenceDiagram
participant User
participant WebPreview
participant iFrame
User->>WebPreview: Click "Zoom In"/"Zoom Out" or "Refresh"
alt Zoom action
WebPreview->>WebPreview: Adjust scale state (between 0.5 and 2)
WebPreview->>iFrame: Update style with new scaling value
else Refresh action
WebPreview->>iFrame: Reset iframe src to refresh content
WebPreview->>WebPreview: Reset scale state to default (0.7)
end
Suggested reviewers
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 ESLint
frontend/src/components/chat/chat-layout.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. frontend/src/app/api/file/route.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team. frontend/src/components/chat/code-engine/project-context.tsxOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the config "next/core-web-vitals" to extend from. Please check that the name of the config is correct. The config "next/core-web-vitals" was referenced from the config file in "/frontend/.eslintrc.json". If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
✨ 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:
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 (
|
2b9e2e3 to
49fc8af
Compare
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: 2
🧹 Nitpick comments (5)
frontend/src/components/chat/code-engine/project-context.tsx (1)
299-299: Remove debug console log statement.This appears to be a debug console log statement that should be removed before merging to production.
- console.log('testing ' + chatId);frontend/src/components/root/root-layout.tsx (1)
45-58: Removed ProjectProvider wrapper - verify context dependencies.The
ProjectProviderwrapper has been removed from around theChatSideBarcomponent. This simplifies the component hierarchy, but please verify that theChatSideBardoesn't need any context previously provided byProjectProvider.Also, the import for
ProjectProvideron line 6 is now unused and can be removed:-import { ProjectProvider } from '@/components/chat/code-engine/project-context';frontend/src/app/api/runProject/route.ts (1)
302-302: Remove explicit 'any' type.The error in the catch block should have a more specific type than 'any'.
- } catch (error: any) { + } catch (error: unknown) { return NextResponse.json( - { error: error.message || 'Failed to start Docker container' }, + { error: error instanceof Error ? error.message : 'Failed to start Docker container' }, { status: 500 } );🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 302-302: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
frontend/src/components/chat/code-engine/code-engine.tsx (2)
24-26: Remove unused props.The
chatIdandprojectIdprops are defined but never used in the component.export function CodeEngine({ - chatId, isProjectReady = false, - projectId, }: { - chatId: string; isProjectReady?: boolean; - projectId?: string; }) {Also applies to: 30-30
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 24-24: 'chatId' is defined but never used. Allowed unused args must match /^_/u @typescript-eslint/no-unused-vars
[warning] 26-26: 'projectId' is defined but never used. Allowed unused args must match /^_/u @typescript-eslint/no-unused-vars
75-76: Improve error type definition.Replace the explicit
anytype with a more specific type orunknown.- } catch (error: any) { - console.error(error.message); + } catch (error: unknown) { + console.error(error instanceof Error ? error.message : String(error)); }🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 75-75: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
[warning] 76-76: Unexpected console statement no-console
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
frontend/src/app/api/file/route.ts(1 hunks)frontend/src/app/api/runProject/route.ts(3 hunks)frontend/src/components/chat/chat-layout.tsx(1 hunks)frontend/src/components/chat/code-engine/code-engine.tsx(3 hunks)frontend/src/components/chat/code-engine/project-context.tsx(1 hunks)frontend/src/components/chat/code-engine/web-view.tsx(8 hunks)frontend/src/components/root/root-layout.tsx(1 hunks)
🧰 Additional context used
🪛 GitHub Actions: autofix.ci
frontend/src/app/api/file/route.ts
[warning] 4-4: 'fs' is defined but never used. Allowed unused vars must match /^_/u @typescript-eslint/no-unused-vars
[warning] 5-5: 'path' is defined but never used. Allowed unused vars must match /^_/u @typescript-eslint/no-unused-vars
[warning] 8-8: Unexpected console statement no-console
[warning] 14-14: Unexpected console statement no-console
[warning] 23-23: Unexpected console statement no-console
[warning] 29-29: Unexpected console statement no-console
[warning] 53-53: 'error' is defined but never used @typescript-eslint/no-unused-vars
frontend/src/app/api/runProject/route.ts
[warning] 84-84: Unexpected console statement no-console
[warning] 87-87: Unexpected console statement no-console
[warning] 90-90: Unexpected console statement no-console
[warning] 131-131: Unexpected console statement no-console
[warning] 138-138: Unexpected console statement no-console
[warning] 142-142: Unexpected console statement no-console
[warning] 143-143: Unexpected console statement no-console
[warning] 153-153: Unexpected console statement no-console
[warning] 158-158: Unexpected console statement no-console
[warning] 168-168: Unexpected console statement no-console
[warning] 173-173: Unexpected console statement no-console
[warning] 182-182: Unexpected console statement no-console
[warning] 194-194: Unexpected console statement no-console
[warning] 204-204: Unexpected console statement no-console
[warning] 214-214: Unexpected console statement no-console
[warning] 275-275: Unexpected console statement no-console
[warning] 277-277: Unexpected console statement no-console
[warning] 302-302: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
frontend/src/components/chat/code-engine/code-engine.tsx
[warning] 24-24: 'chatId' is defined but never used. Allowed unused args must match /^_/u @typescript-eslint/no-unused-vars
[warning] 26-26: 'projectId' is defined but never used. Allowed unused args must match /^_/u @typescript-eslint/no-unused-vars
[warning] 42-42: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
[warning] 75-75: Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
[warning] 76-76: Unexpected console statement no-console
[warning] 86-86: Unexpected console statement no-console
[warning] 94-94: Unexpected console statement no-console
[warning] 98-98: Unexpected console statement no-console
[error] 127-127: 'iframe.src' is assigned to itself no-self-assign
🪛 Biome (1.9.4)
frontend/src/components/chat/code-engine/web-view.tsx
[error] 140-140: src is assigned to itself.
This is where is assigned.
(lint/correctness/noSelfAssign)
🪛 GitHub Check: autofix
frontend/src/components/chat/code-engine/web-view.tsx
[failure] 140-140:
'iframe.src' is assigned to itself
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Run tests and collect coverage
🔇 Additional comments (20)
frontend/src/app/api/file/route.ts (1)
62-63:❓ Verification inconclusive
Good addition for
.tsxfile type support.Adding support for TypeScript React files (.tsx) is valuable as it properly identifies these files for the editor.
However, there are several linting issues flagged by the pipeline that should be addressed:
- Unused imports:
fsandpathon lines 4-5- Console statements throughout the file
- Unused variable:
erroron line 53
🏁 Script executed:
#!/bin/bash # Check if there are other places in the codebase that might be using typescriptreact extension # This helps verify if the implementation is consistent # Search for other typeMap definitions rg "typeMap.*typescriptreact" --type ts --type tsxLength of output: 80
Attention: Review Pending for Lint Fixes & Search Verification
- The addition of support for
.tsxfiles by mapping them totypescriptreactis a positive improvement.- Please address the following linting issues:
- Remove unused imports (
fsandpathon lines 4–5).- Eliminate console statements throughout the file.
- Remove the unused variable
erroron line 53.- Verification Note: The current
rgcommand produced the errorunrecognized file type: tsx. This indicates thattsxis not a recognized file type by default inrg. Consider updating the command (e.g., adding--type-add "tsx:*.tsx") or perform manual verification to ensure consistent usage of thetypescriptreactmapping across the codebase.frontend/src/components/chat/code-engine/project-context.tsx (1)
116-116:❓ Verification inconclusive
Update navigation pattern for consistency across project actions.
You've changed project creation navigation to use query parameters (
/chat?id=${id}), but the fork project navigation on line 129 still uses path parameters (/chat/${id}). This inconsistency may cause issues.Apply this change to make navigation consistent:
-router.push(`/chat/${data.forkProject.id}`); +router.push(`/chat?id=${data.forkProject.id}`);
🏁 Script executed:
#!/bin/bash # Check if there are other places in the codebase that might be navigating to /chat with an ID # This helps ensure all navigation is consistent with the new pattern # Search for other router.push calls to chat paths rg "router\.push\(\`?['\"]?\/chat\/" --type ts --type tsxLength of output: 177
Critical: Ensure Navigation Consistency for Chat Routes
It appears that project creation currently navigates using query parameters (i.e.,
/chat?id=${data.createProject.id}) while the fork project navigation still uses path parameters (i.e.,/chat/${data.forkProject.id}). For consistency and to avoid potential issues, please update the fork project navigation as follows:-router.push(`/chat/${data.forkProject.id}`); +router.push(`/chat?id=${data.forkProject.id}`);Additionally, to ensure there are no other instances of navigation using
/chat/with path parameters, please run the following shell script to verify the codebase:#!/bin/bash # Search for any router.push calls using path parameters for /chat/ rg -n "router\.push\(\s*`\/chat\/" --type ts --type tsxPlease review the output of this script manually to confirm that no other occurrences exist.
frontend/src/app/api/runProject/route.ts (5)
131-133: Added helpful logging for Docker build start.This log provides better visibility into the Docker build process, making it easier to track progress and diagnose issues.
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 131-131: Unexpected console statement no-console
147-151: Improved Docker run command formatting.The reformatting of the Docker run command enhances readability while maintaining functionality.
153-153: Added command execution logging.Logging the full run command provides valuable information for debugging Docker-related issues.
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 153-153: Unexpected console statement no-console
158-202: Enhanced error handling for Docker container name conflicts.The implementation now properly handles container name conflicts by removing the existing container and retrying the operation, with appropriate logging at each step. This makes the Docker container management more robust.
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 158-158: Unexpected console statement no-console
[warning] 168-168: Unexpected console statement no-console
[warning] 173-173: Unexpected console statement no-console
[warning] 182-182: Unexpected console statement no-console
[warning] 194-194: Unexpected console statement no-console
252-282: Improved container status verification.The code now properly checks if the existing container is actually running using
docker inspectbefore deciding whether to reuse it or remove and recreate it. This addresses potential issues where stopped containers were being reported as available.🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 275-275: Unexpected console statement no-console
[warning] 277-277: Unexpected console statement no-console
frontend/src/components/chat/code-engine/code-engine.tsx (5)
85-88: Added missing project path logging.The enhanced error logging helps identify when a project path is missing, which is a common cause of issues when loading projects.
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 86-86: Unexpected console statement no-console
94-94: Added file structure loading log.This log helps track the progress of loading the file structure, which is useful for debugging.
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 94-94: Unexpected console statement no-console
277-277: Changed overflow behavior to enable scrolling.Switching from
overflow-hiddentooverflow-scrollallows users to scroll through content that exceeds the container dimensions, improving usability.
282-282: Adjusted height calculation for better layout.The change from
calc(100vh-48px-2rem)tocalc(100vh-48px-4rem)allows for more spacing, which may improve the content layout.
318-318: Updated default editor language.Changing the default language to
typescriptreactbetter aligns with the expected code type, especially if users are primarily working with React components.frontend/src/components/chat/code-engine/web-view.tsx (8)
10-12: Added new UI control icons.The inclusion of these icons supports the new refresh and zoom functionality.
21-21: Added scale state for zoom functionality.This new state variable enables the zoom in/out feature for the preview iframe.
56-56: Reduced initial delay.Reducing the timeout from 200ms to 100ms might make the preview appear more responsive.
63-82: Implemented URL status polling.The new
checkUrlStatusfunction repeatedly attempts to connect to the URL until it returns a 200 status code, which ensures the preview is available before displaying it to users.
145-151: Added zoom controls with reasonable limits.The implementation includes reasonable bounds (0.5 to 2.0) for the zoom scale, preventing users from zooming too far in or out.
177-185: Added refresh button.This new control allows users to reload the iframe content, which is useful when testing changes to the application.
201-219: Added zoom controls.These buttons provide an intuitive way to adjust the preview size, enhancing the user experience.
244-254: Implemented iframe scaling.The changes provide a smooth scaling implementation for the iframe, using CSS transform and proper width/height adjustments to ensure correct rendering at different zoom levels.
| <ProjectModal | ||
| isOpen={isModalOpen} | ||
| onClose={() => setIsModalOpen(false)} | ||
| refetchProjects={refetch} | ||
| /> | ||
| <div className="w-full h-full">{children}</div> |
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.
💡 Verification agent
🧩 Analysis chain
Removed ProjectProvider wrapper - verify context dependencies.
You've removed the ProjectProvider wrapper from around ProjectModal and its children. This simplifies the component hierarchy, but ensure that ProjectModal and other child components don't depend on context data previously provided by ProjectProvider.
🏁 Script executed:
#!/bin/bash
# Check if ProjectModal uses any context from ProjectContext
ast-grep --pattern 'useContext(ProjectContext)' frontend/src/components/chat/project-modal.tsx || echo "ProjectContext not directly used in ProjectModal"
# Check for imports of ProjectContext in the modal
rg "import.*ProjectContext" frontend/src/components/chat/project-modal.tsxLength of output: 363
Action Required: Address ProjectModal's Context Dependency
- The
ProjectModalcomponent (atfrontend/src/components/chat/project-modal.tsx) still usesuseContext(ProjectContext)(line 24), and imports{ ProjectContext, ProjectProvider }. - With the removal of the
ProjectProviderwrapper inchat-layout.tsx,ProjectModalwill no longer receive the expected context, which could lead to runtime errors or unexpected behavior. - Next Steps:
- Either reintroduce the
ProjectProviderwrapper aroundProjectModaland its children, or refactorProjectModalto remove its reliance onProjectContext(or provide default values/props as needed). - Verify all components consuming
ProjectContextare updated consistently throughout the codebase.
- Either reintroduce the
| const reloadIframe = () => { | ||
| const iframe = document.getElementById('myIframe') as HTMLIFrameElement; | ||
| if (iframe) { | ||
| iframe.src = iframe.src; | ||
| setScale(0.7); | ||
| } | ||
| }; |
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.
Fix self-assignment issue in reload function.
The current implementation assigns iframe.src to itself, which doesn't actually reload the iframe.
const reloadIframe = () => {
const iframe = document.getElementById('myIframe') as HTMLIFrameElement;
if (iframe) {
- iframe.src = iframe.src;
+ const currentSrc = iframe.src;
+ iframe.src = '';
+ setTimeout(() => {
+ iframe.src = currentSrc;
+ }, 10);
setScale(0.7);
}
};📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const reloadIframe = () => { | |
| const iframe = document.getElementById('myIframe') as HTMLIFrameElement; | |
| if (iframe) { | |
| iframe.src = iframe.src; | |
| setScale(0.7); | |
| } | |
| }; | |
| const reloadIframe = () => { | |
| const iframe = document.getElementById('myIframe') as HTMLIFrameElement; | |
| if (iframe) { | |
| const currentSrc = iframe.src; | |
| iframe.src = ''; | |
| setTimeout(() => { | |
| iframe.src = currentSrc; | |
| }, 10); | |
| setScale(0.7); | |
| } | |
| }; |
🧰 Tools
🪛 Biome (1.9.4)
[error] 140-140: src is assigned to itself.
This is where is assigned.
(lint/correctness/noSelfAssign)
🪛 GitHub Check: autofix
[failure] 140-140:
'iframe.src' is assigned to itself
…e project from home page (#151) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced file recognition to support .tsx files as TypeScript React. - Introduced new iframe controls, allowing users to refresh content and adjust zoom levels. - Updated the code editor’s default language for a more accurate editing experience. - Revised project navigation to use query parameters for improved clarity. - **Bug Fixes** - Improved Docker container management with enhanced logging, error handling, and retry mechanisms. - **Refactor** - Simplified chat and sidebar structures by removing unnecessary context providers. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Summary by CodeRabbit
New Features
Bug Fixes
Refactor