feat(frontend): implement expandable project cards and fetch public p… #152
Conversation
WalkthroughThis PR introduces a new React component called Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant EC as ExpandableCard
participant API as getWebUrl API
participant Hook as useOutsideClick
U->>EC: Clicks on project card
EC->>API: Calls getWebUrl(projectId)
API-->>EC: Returns project URL
EC->>U: Opens modal with iframe view
U->>Hook: Clicks outside modal or presses 'Escape'
Hook->>EC: Triggers callback to close modal
EC->>U: Closes modal, resets active project
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
💤 Files with no reviewable changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (6)
frontend/src/hooks/use-outside-click.tsx (1)
10-10: Add type assertion for event.targetWhen using more specific event types, you'll need to add a type assertion for
event.targetwhen checking if it's contained within the ref.- if (!ref.current || ref.current.contains(event.target)) { + if (!ref.current || ref.current.contains(event.target as Node)) {frontend/src/components/root/expand-card.tsx (5)
9-10: Add proper typing for state variablesState variables should have explicit type definitions to improve type safety.
- const [active, setActive] = useState(null); - const [iframeUrl, setIframeUrl] = useState(''); + const [active, setActive] = useState<Project | null>(null); + const [iframeUrl, setIframeUrl] = useState<string>('');
35-36: Remove console.log statementConsole logging statements should be removed from production code.
- console.log('project:', project);
110-114: Add error handling and loading state for iframeAdd error handling for the iframe and display loading state to improve user experience.
- <iframe - src={iframeUrl} - className="w-full h-[100%]" - title="Project Preview" - /> + {isLoading ? ( + <div className="w-full h-full flex items-center justify-center"> + <div className="animate-spin rounded-full h-12 w-12 border-t-2 border-b-2 border-primary-600"></div> + </div> + ) : error ? ( + <div className="w-full h-full flex items-center justify-center text-red-500"> + {error} + </div> + ) : ( + <iframe + src={iframeUrl} + className="w-full h-[100%]" + title="Project Preview" + onError={() => setError('Failed to load project preview.')} + sandbox="allow-scripts allow-same-origin" + /> + )}
21-25: Simplify active state checkThe current check for active state is unnecessarily complex. Since we're now typing
activeasProject | null, we can simplify this check.- if (active && typeof active === 'object') { + if (active) { document.body.style.overflow = 'hidden'; } else { document.body.style.overflow = 'auto'; }
123-173: Add accessibility attributes for better user experienceThe project cards should have proper accessibility attributes for better user experience, especially for screen readers.
Consider adding the following improvements:
- Add
aria-labelattributes to clickable elements- Use semantic HTML elements where appropriate
- Ensure proper keyboard navigation
Here's an example for the project card:
onClick={() => getWebUrl(project)} - className="group cursor-pointer" + className="group cursor-pointer" + role="button" + aria-label={`View project ${project.name}`} + tabIndex={0} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + getWebUrl(project); + } + }}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
frontend/src/components/root/expand-card.tsx(1 hunks)frontend/src/components/root/projects-section.tsx(3 hunks)frontend/src/graphql/request.ts(1 hunks)frontend/src/graphql/schema.gql(2 hunks)frontend/src/hooks/use-outside-click.tsx(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- frontend/src/graphql/schema.gql
🧰 Additional context used
🪛 Biome (1.9.4)
frontend/src/hooks/use-outside-click.tsx
[error] 5-5: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🪛 GitHub Actions: autofix.ci
frontend/src/hooks/use-outside-click.tsx
[error] 5-5: The Function type accepts any function-like value. Prefer explicitly defining any function parameters and return type.
[warning] 8-8: Unexpected any. Specify a different type.
🔇 Additional comments (3)
frontend/src/graphql/request.ts (1)
20-34: New GraphQL query implementation looks goodThe implementation of the
FETCH_PUBLIC_PROJECTSquery is clean and well-structured, clearly defining the fields needed for the public projects feature.frontend/src/components/root/projects-section.tsx (2)
1-7: Imports are well organizedThe imports are well structured, bringing in the required components and functions for the implementation.
19-30: Proper data transformation logicThe transformation of fetched data into the format expected by the ExpandableCard component is well implemented. Adding the
pathproperty ensures the component has all the necessary data.
| callback: Function | ||
| ) => { | ||
| useEffect(() => { | ||
| const listener = (event: any) => { |
There was a problem hiding this comment.
Replace any type with a more specific event type
Using any for the event parameter reduces type safety and is flagged in the pipeline failures.
- const listener = (event: any) => {
+ const listener = (event: MouseEvent | TouchEvent) => {📝 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 listener = (event: any) => { | |
| const listener = (event: MouseEvent | TouchEvent) => { |
🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 8-8: Unexpected any. Specify a different type.
| export const useOutsideClick = ( | ||
| ref: React.RefObject<HTMLDivElement>, | ||
| callback: Function | ||
| ) => { |
There was a problem hiding this comment.
Replace Function type with a specific function signature
The Function type is too generic and can lead to type-related bugs. This is causing pipeline failures as shown in the static analysis hints.
-export const useOutsideClick = (
- ref: React.RefObject<HTMLDivElement>,
- callback: Function
-) => {
+export const useOutsideClick = (
+ ref: React.RefObject<HTMLDivElement>,
+ callback: (event: MouseEvent | TouchEvent) => void
+) => {📝 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.
| export const useOutsideClick = ( | |
| ref: React.RefObject<HTMLDivElement>, | |
| callback: Function | |
| ) => { | |
| export const useOutsideClick = ( | |
| ref: React.RefObject<HTMLDivElement>, | |
| callback: (event: MouseEvent | TouchEvent) => void | |
| ) => { |
🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🪛 GitHub Actions: autofix.ci
[error] 5-5: The Function type accepts any function-like value. Prefer explicitly defining any function parameters and return type.
| <ProjectCard key={project.id} project={project} /> | ||
| ))} | ||
| </div> | ||
| <ExpandableCard projects={transformedProjects} /> |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Missing type definition for props
The ExpandableCard component doesn't have TypeScript type definitions, which could lead to runtime errors if the wrong data structure is passed.
🏁 Script executed:
#!/bin/bash
# Check if the ExpandableCard component has TypeScript type definitions in its implementation
rg -t tsx "export function ExpandableCard\(.+\): " --include=./frontend/src/components/root/expand-card.tsxLength of output: 551
I'll re-run the verification using a revised shell script that avoids the unsupported --include flag and also searches for any type/interface definitions (e.g., ExpandableCardProps) in the file. This should help us confirm whether the ExpandableCard component has TypeScript props definitions.
🏁 Script executed:
#!/bin/bash
echo "Searching for the ExpandableCard component definition in expand-card.tsx..."
rg -t tsx "export function ExpandableCard\\(" frontend/src/components/root/expand-card.tsx
echo ""
echo "Searching for a type or interface definition for the ExpandableCard props (e.g., ExpandableCardProps)..."
rg -t tsx "(interface|type) ExpandableCardProps" frontend/src/components/root/expand-card.tsxLength of output: 634
Action Required: Add Type Definitions for ExpandableCard Props
Our verification checks indicate that the ExpandableCard component in frontend/src/components/root/expand-card.tsx does not include an explicit type or interface for its props. This omission can lead to runtime errors if an unexpected data structure is passed. Please add a proper type definition (for example, by defining an interface such as ExpandableCardProps) to ensure type safety.
- Location:
frontend/src/components/root/expand-card.tsx
| import { useOutsideClick } from '@/hooks/use-outside-click'; | ||
| import { X } from 'lucide-react'; | ||
|
|
||
| export function ExpandableCard({ projects }) { |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add TypeScript interface for projects prop
The component accepts a projects prop without type definition, which reduces type safety and could lead to runtime errors.
-export function ExpandableCard({ projects }) {
+interface Project {
+ id: string;
+ name: string;
+ path: string;
+ createDate: string;
+ author: string;
+ forkNum: number;
+ image: string;
+}
+
+interface ExpandableCardProps {
+ projects: Project[];
+}
+
+export function ExpandableCard({ projects }: ExpandableCardProps) {📝 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.
| export function ExpandableCard({ projects }) { | |
| interface Project { | |
| id: string; | |
| name: string; | |
| path: string; | |
| createDate: string; | |
| author: string; | |
| forkNum: number; | |
| image: string; | |
| } | |
| interface ExpandableCardProps { | |
| projects: Project[]; | |
| } | |
| export function ExpandableCard({ projects }: ExpandableCardProps) { |
| try { | ||
| const response = await fetch( | ||
| `/api/runProject?projectPath=${encodeURIComponent(projectPath)}`, | ||
| { | ||
| method: 'GET', | ||
| headers: { | ||
| 'Content-Type': 'application/json', | ||
| }, | ||
| } | ||
| ); | ||
| const json = await response.json(); | ||
| const baseUrl = `http://${json.domain}`; | ||
| setIframeUrl(baseUrl); | ||
| setActive(project); | ||
| } catch (error) { | ||
| console.error('fetching url error:', error); | ||
| } | ||
| }; |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and add loading state for URL fetching
The current implementation only logs errors to the console without providing user feedback. Add proper error handling and loading state to improve the user experience.
+ const [isLoading, setIsLoading] = useState(false);
+ const [error, setError] = useState<string | null>(null);
const getWebUrl = async (project) => {
if (!project) return;
- console.log('project:', project);
const projectPath = project.path;
+ setIsLoading(true);
+ setError(null);
try {
const response = await fetch(
`/api/runProject?projectPath=${encodeURIComponent(projectPath)}`,
{
method: 'GET',
headers: {
'Content-Type': 'application/json',
},
}
);
+ if (!response.ok) {
+ throw new Error(`Failed to fetch project URL (${response.status})`);
+ }
const json = await response.json();
const baseUrl = `http://${json.domain}`;
setIframeUrl(baseUrl);
setActive(project);
} catch (error) {
console.error('fetching url error:', error);
+ setError('Failed to load project preview. Please try again.');
+ } finally {
+ setIsLoading(false);
}
};📝 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.
| try { | |
| const response = await fetch( | |
| `/api/runProject?projectPath=${encodeURIComponent(projectPath)}`, | |
| { | |
| method: 'GET', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| const json = await response.json(); | |
| const baseUrl = `http://${json.domain}`; | |
| setIframeUrl(baseUrl); | |
| setActive(project); | |
| } catch (error) { | |
| console.error('fetching url error:', error); | |
| } | |
| }; | |
| const [isLoading, setIsLoading] = useState(false); | |
| const [error, setError] = useState<string | null>(null); | |
| const getWebUrl = async (project) => { | |
| if (!project) return; | |
| const projectPath = project.path; | |
| setIsLoading(true); | |
| setError(null); | |
| try { | |
| const response = await fetch( | |
| `/api/runProject?projectPath=${encodeURIComponent(projectPath)}`, | |
| { | |
| method: 'GET', | |
| headers: { | |
| 'Content-Type': 'application/json', | |
| }, | |
| } | |
| ); | |
| if (!response.ok) { | |
| throw new Error(`Failed to fetch project URL (${response.status})`); | |
| } | |
| const json = await response.json(); | |
| const baseUrl = `http://${json.domain}`; | |
| setIframeUrl(baseUrl); | |
| setActive(project); | |
| } catch (error) { | |
| console.error('fetching url error:', error); | |
| setError('Failed to load project preview. Please try again.'); | |
| } finally { | |
| setIsLoading(false); | |
| } | |
| }; |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
frontend/src/components/root/expand-card.tsx (2)
7-7: 🛠️ Refactor suggestionAdd TypeScript interface for projects prop
The component accepts a
projectsprop without type definition, which reduces type safety and could lead to runtime errors.-export function ExpandableCard({ projects }) { +interface Project { + id: string; + name: string; + path: string; + createDate: string; + author: string; + forkNum: number; + image: string; +} + +interface ExpandableCardProps { + projects: Project[]; +} + +export function ExpandableCard({ projects }: ExpandableCardProps) {
29-51: 🛠️ Refactor suggestionImprove error handling and add loading state for URL fetching
The current implementation only logs errors to the console without providing user feedback. Add proper error handling and loading state to improve the user experience.
+ const [isLoading, setIsLoading] = useState(false); + const [error, setError] = useState<string | null>(null); const getWebUrl = async (project) => { if (!project) return; - console.log('project:', project); const projectPath = project.path; + setIsLoading(true); + setError(null); try { const response = await fetch( `/api/runProject?projectPath=${encodeURIComponent(projectPath)}`, { method: 'GET', headers: { 'Content-Type': 'application/json', }, } ); + if (!response.ok) { + throw new Error(`Failed to fetch project URL (${response.status})`); + } const json = await response.json(); const baseUrl = `http://${json.domain}`; setIframeUrl(baseUrl); setActive(project); } catch (error) { console.error('fetching url error:', error); + setError('Failed to load project preview. Please try again.'); + } finally { + setIsLoading(false); } };🧰 Tools
🪛 GitHub Actions: autofix.ci
[warning] 31-31: Unexpected console statement no-console
🧹 Nitpick comments (5)
frontend/src/components/root/expand-card.tsx (3)
12-27: Add a useOutsideClick custom hook for better modal interactionThe current implementation handles the Escape key correctly, but best practices for modals also include closing when clicking outside the modal content. This improves user experience.
+// Add this hook at the top of the file or in a separate hooks directory +function useOutsideClick(ref, callback) { + useEffect(() => { + function handleClickOutside(event) { + if (ref.current && !ref.current.contains(event.target)) { + callback(); + } + } + + document.addEventListener('mousedown', handleClickOutside); + return () => { + document.removeEventListener('mousedown', handleClickOutside); + }; + }, [ref, callback]); +} useEffect(() => { function onKeyDown(event: KeyboardEvent) { if (event.key === 'Escape') { setActive(null); } } if (active && typeof active === 'object') { document.body.style.overflow = 'hidden'; } else { document.body.style.overflow = 'auto'; } window.addEventListener('keydown', onKeyDown); return () => window.removeEventListener('keydown', onKeyDown); }, [active]); +// Then use it with your ref +useOutsideClick(ref, () => active && setActive(null));
86-91: Add aria attributes for improved accessibilityThe modal lacks proper accessibility attributes, which can make it difficult for screen reader users to navigate.
<motion.div layoutId={`card-${active.id}`} ref={ref} className="w-full h-full flex flex-col bg-white dark:bg-neutral-900 rounded-2xl overflow-hidden" style={{ willChange: 'transform, opacity' }} + role="dialog" + aria-modal="true" + aria-labelledby={`project-title-${active.id}`} >Then update the title element:
<motion.h3 layoutId={`title-${active.id}`} className="text-xl font-semibold text-gray-900 dark:text-gray-100" + id={`project-title-${active.id}`} >
125-127: Add keyboard accessibility for project cardsThe project cards are not accessible via keyboard navigation. Add proper keyboard support to improve accessibility.
<motion.div key={project.id} layoutId={`card-${project.id}`} onClick={() => getWebUrl(project)} + onKeyDown={(e) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + getWebUrl(project); + } + }} + tabIndex={0} + role="button" + aria-label={`View ${project.name} project`} className="group cursor-pointer" >frontend/src/components/root/projects-section.tsx (2)
48-48: Add error boundary for the ExpandableCard componentThe ExpandableCard component might throw errors during rendering or when fetching project URLs. Wrapping it in an error boundary would prevent the entire page from crashing.
+// Add at the top of the file +import { ErrorBoundary } from 'react-error-boundary'; + +function FallbackComponent({ error, resetErrorBoundary }) { + return ( + <div className="text-center py-4 px-6 bg-red-50 rounded-lg border border-red-200 dark:bg-red-900/20 dark:border-red-800"> + <h3 className="text-lg font-medium text-red-800 dark:text-red-200">Something went wrong:</h3> + <p className="text-sm text-red-600 dark:text-red-300 mt-2">{error.message}</p> + <button + onClick={resetErrorBoundary} + className="mt-4 px-4 py-2 bg-red-600 text-white rounded-md hover:bg-red-700 transition-colors" + > + Try again + </button> + </div> + ); +} // Then in the JSX - <ExpandableCard projects={transformedProjects} /> + <ErrorBoundary + FallbackComponent={FallbackComponent} + onReset={() => { + // Reset the state that caused the error + }} + > + <ExpandableCard projects={transformedProjects} /> + </ErrorBoundary>
41-45: Improve loading and error statesThe current loading and error states are very basic. Enhancing them with better visual feedback would improve the user experience.
{loading ? ( - <div className="text-center py-10">Loading...</div> + <div className="text-center py-10"> + <div className="inline-block animate-spin rounded-full h-8 w-8 border-b-2 border-primary-600 mb-2"></div> + <p className="text-gray-600 dark:text-gray-400">Loading projects...</p> + </div> ) : error ? ( - <div className="text-center py-10">Error: {error.message}</div> + <div className="text-center py-10 bg-red-50 rounded-lg p-4 dark:bg-red-900/20"> + <div className="flex items-center justify-center mb-2"> + <svg className="w-6 h-6 text-red-600 dark:text-red-400" fill="none" stroke="currentColor" viewBox="0 0 24 24" xmlns="http://www.w3.org/2000/svg"> + <path strokeLinecap="round" strokeLinejoin="round" strokeWidth={2} d="M12 9v2m0 4h.01m-6.938 4h13.856c1.54 0 2.502-1.667 1.732-3L13.732 4c-.77-1.333-2.694-1.333-3.464 0L3.34 16c-.77 1.333.192 3 1.732 3z" /> + </svg> + </div> + <p className="text-red-800 dark:text-red-200 font-medium">Unable to load projects</p> + <p className="text-red-600 dark:text-red-300 text-sm mt-1">{error.message}</p> + <button + onClick={() => window.location.reload()} + className="mt-4 px-4 py-2 bg-red-600 text-white rounded-md hover:bg-red-700 transition-colors" + > + Refresh + </button> + </div> ) : (
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/root/expand-card.tsx(1 hunks)frontend/src/components/root/projects-section.tsx(3 hunks)frontend/src/graphql/request.ts(1 hunks)frontend/src/graphql/schema.gql(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/graphql/request.ts
- frontend/src/graphql/schema.gql
🧰 Additional context used
🪛 GitHub Actions: autofix.ci
frontend/src/components/root/expand-card.tsx
[warning] 31-31: Unexpected console statement no-console
🔇 Additional comments (2)
frontend/src/components/root/projects-section.tsx (2)
1-3: LGTM! Good import organization.The imports are well-organized, with the GraphQL query now imported from a dedicated file and the new ExpandableCard component properly imported.
18-18: Good addition of the path propertyThe path property is correctly added to the transformed project data, which is essential for the ExpandableCard component to fetch project details.
| <iframe | ||
| src={iframeUrl} | ||
| className="w-full h-[100%]" | ||
| title="Project Preview" | ||
| /> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Enhance iframe security and add loading state
The iframe implementation lacks security attributes and doesn't show any loading indicator while the content is being fetched.
<iframe
src={iframeUrl}
className="w-full h-[100%]"
title="Project Preview"
+ sandbox="allow-scripts allow-same-origin"
+ loading="lazy"
+ referrerPolicy="no-referrer"
/>
+ {isLoading && (
+ <div className="absolute inset-0 flex items-center justify-center bg-white/80 dark:bg-black/80">
+ <div className="animate-spin rounded-full h-10 w-10 border-b-2 border-primary-600"></div>
+ </div>
+ )}
+ {error && (
+ <div className="absolute inset-0 flex items-center justify-center bg-white/90 dark:bg-black/90">
+ <div className="text-red-500 text-center p-4">
+ <p>{error}</p>
+ <button
+ className="mt-4 px-4 py-2 bg-primary-600 text-white rounded-lg"
+ onClick={(e) => {
+ e.stopPropagation();
+ getWebUrl(active);
+ }}
+ >
+ Retry
+ </button>
+ </div>
+ </div>
+ )}📝 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.
| <iframe | |
| src={iframeUrl} | |
| className="w-full h-[100%]" | |
| title="Project Preview" | |
| /> | |
| <iframe | |
| src={iframeUrl} | |
| className="w-full h-[100%]" | |
| title="Project Preview" | |
| sandbox="allow-scripts allow-same-origin" | |
| loading="lazy" | |
| referrerPolicy="no-referrer" | |
| /> | |
| {isLoading && ( | |
| <div className="absolute inset-0 flex items-center justify-center bg-white/80 dark:bg-black/80"> | |
| <div className="animate-spin rounded-full h-10 w-10 border-b-2 border-primary-600"></div> | |
| </div> | |
| )} | |
| {error && ( | |
| <div className="absolute inset-0 flex items-center justify-center bg-white/90 dark:bg-black/90"> | |
| <div className="text-red-500 text-center p-4"> | |
| <p>{error}</p> | |
| <button | |
| className="mt-4 px-4 py-2 bg-primary-600 text-white rounded-lg" | |
| onClick={(e) => { | |
| e.stopPropagation(); | |
| getWebUrl(active); | |
| }} | |
| > | |
| Retry | |
| </button> | |
| </div> | |
| </div> | |
| )} |
…rojects query
Summary by CodeRabbit
New Features
ExpandableCardcomponent for project previews, featuring a modal overlay with smooth animations.Refactor
Chores
SidebarPropsinterface and theChatSideBarcomponent for a cleaner codebase.