feat(projects): tab navigation on project show page - #4
Merged
Conversation
…s tabs Split project detail content into three tabs to improve scannability. Lazy-load comments on tab activation instead of on page load.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR converts the project show page from a static layout into a tabbed interface with three tabs: team (equipo), activity (actividad), and comments (comentarios). Tab labels are translated in both English and Spanish, tab state drives conditional rendering of page sections, and comment loading is deferred until the comments tab is selected. ChangesTabbed Project Show Page
Sequence DiagramsequenceDiagram
participant User
participant TabNav as Tab Navigation
participant ShowPage as ProyectoShow Component
participant Comments as Comments Loader
User->>TabNav: Click "comentarios" tab
TabNav->>ShowPage: setActiveTab("comentarios")
ShowPage->>Comments: useEffect triggers loadComments()
Comments->>Comments: fetch comments
Comments-->>ShowPage: update commentsList & commentsLoading
ShowPage-->>User: render PostComments or skeleton
🎯 3 (Moderate) | ⏱️ ~20 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
resources/js/pages/proyectos/show.tsx (1)
119-126: ⚡ Quick winAdd missing dependency to useEffect or refactor to silence exhaustive-deps.
The effect calls
loadComments()but doesn't include it in the dependency array. While the code works due to the internal guard inloadComments, this violates the React exhaustive-deps rule and may trigger lint warnings.♻️ Refactor to move logic inline
useEffect(() => { if (activeTab === 'comentarios') { - loadComments().catch(() => { - setCommentsLoading(false); - setCommentsList([]); - }); + (async () => { + if (commentsList !== null) return; + setCommentsLoading(true); + try { + const response = await fetch(fetchComments.url(project.id)); + const data: Comment[] = await response.json(); + setCommentsList(data); + } catch { + setCommentsList([]); + } finally { + setCommentsLoading(false); + } + })(); } -}, [activeTab]); +}, [activeTab, commentsList, project.id]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/pages/proyectos/show.tsx` around lines 119 - 126, The useEffect references loadComments but doesn't include it in the dependency array causing exhaustive-deps warnings; fix by either adding loadComments to the dependency array or refactoring the effect to declare the async logic inline (e.g., create an async function inside the effect that calls loadComments() logic and uses setCommentsLoading/setCommentsList) or wrap loadComments in useCallback so it can be safely added to deps; update the effect to depend on [activeTab, loadComments] if you choose to add it, or replace the call with the inline async handler to silence the lint rule.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@resources/js/pages/proyectos/show.tsx`:
- Around line 119-126: The useEffect references loadComments but doesn't include
it in the dependency array causing exhaustive-deps warnings; fix by either
adding loadComments to the dependency array or refactoring the effect to declare
the async logic inline (e.g., create an async function inside the effect that
calls loadComments() logic and uses setCommentsLoading/setCommentsList) or wrap
loadComments in useCallback so it can be safely added to deps; update the effect
to depend on [activeTab, loadComments] if you choose to add it, or replace the
call with the inline async handler to silence the lint rule.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: de0ef590-c456-44db-b5ce-b6da616679ad
📒 Files selected for processing (4)
lang/en/projects.phplang/es/projects.phpresources/js/pages/proyectos/show.tsxtests/Browser/ProjectTimelineComposerTest.php
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
projects.show.tabs.*i18n keys to bothesandenlang filesTest plan
/projects/:id— header, gallery visible; tab bar shows Equipo / Actividad / ComentariosSummary by CodeRabbit
New Features
Behavior Changes
Localization
Tests