Skip to content

refactor the cardDetailsPage #2440

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.

Sign up for GitHub

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

Closed
Rakesh-18112006 wants to merge 2 commits into from
Closed

refactor the cardDetailsPage #2440

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
407b9ac
refactor the cardDetailsPage
Rakesh-18112006 Oct 17, 2025
bbe622a
Merge branch 'main' into issue#2414
Rakesh-18112006 Oct 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion frontend/__tests__/e2e/components/Footer.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ test.describe('Footer - Desktop (Chrome)', () => {
})

// Mobile tests (iPhone 13)
test.use({
test.use({
...devices['iPhone 13'],
isMobile: true,
})
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/app/mentorship/programs/[programKey]/modules/[moduleKey]/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { formatDate } from 'utils/dateFormatter'
import DetailsCard from 'components/CardDetailsPage'
import LoadingSpinner from 'components/LoadingSpinner'
import { getSimpleDuration } from 'components/ModuleCard'

const ModuleDetailsPage = () => {
const { programKey, moduleKey } = useParams<{ programKey: string; moduleKey: string }>()
const [module, setModule] = useState<Module | null>(null)
Expand Down
56 changes: 56 additions & 0 deletions frontend/src/components/ActivitySection.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
import RecentIssues from 'components/RecentIssues'
import RecentPullRequests from 'components/RecentPullRequests'
import RecentReleases from 'components/RecentReleases'
import Milestones from 'components/Milestones'
import type { Issue } from 'types/issue'
import type { PullRequest } from 'types/pullRequest'
import type { Milestone } from 'types/milestone'
import type { Release } from 'types/release'

interface ActivitySectionProps {
type: string
recentIssues?: Issue[]
pullRequests?: PullRequest[]
recentMilestones?: Milestone[]
recentReleases?: Release[]
showAvatar?: boolean
}
Comment on lines +10 to +17
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion | 🟠 Major

Remove duplicate type definition.

ActivitySectionProps is defined locally but also exists in types/components.ts (per relevant snippets). The two definitions differ: the local version marks arrays as optional, while types/components.ts defines them as required. This duplication creates maintenance overhead and type inconsistency risk.

Remove the local interface and import from the shared types file:

+import type { ActivitySectionProps } from 'types/components'
 import type { Issue } from 'types/issue'
 import type { PullRequest } from 'types/pullRequest'
 import type { Milestone } from 'types/milestone'
 import type { Release } from 'types/release'

-interface ActivitySectionProps {
-  type: string
-  recentIssues?: Issue[]
-  pullRequests?: PullRequest[]
-  recentMilestones?: Milestone[]
-  recentReleases?: Release[]
-  showAvatar?: boolean
-}
📝 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.

Suggested change
interface ActivitySectionProps {
type: string
recentIssues?: Issue[]
pullRequests?: PullRequest[]
recentMilestones?: Milestone[]
recentReleases?: Release[]
showAvatar?: boolean
}
import type { ActivitySectionProps } from 'types/components'
import type { Issue } from 'types/issue'
import type { PullRequest } from 'types/pullRequest'
import type { Milestone } from 'types/milestone'
import type { Release } from 'types/release'
🤖 Prompt for AI Agents 
In frontend/src/components/ActivitySection.tsx around lines 10-17, remove the
local ActivitySectionProps interface declaration and instead import the shared
type from the central types file (e.g. import { ActivitySectionProps } from
'types/components' or the correct relative path); ensure the component uses that
imported type (or Partial<ActivitySectionProps> if you intentionally need
optional arrays) and update any local references so there are no
duplicate/conflicting definitions.


const ActivitySection = ({
type,
recentIssues,
pullRequests,
recentMilestones,
recentReleases,
showAvatar,
}: ActivitySectionProps) => (
<>
{(type === 'project' ||
type === 'repository' ||
type === 'user' ||
type === 'organization') && (
<div className="grid-cols-2 gap-4 lg:grid">
<RecentIssues data={recentIssues} showAvatar={showAvatar} />
{type === 'user' ||
type === 'organization' ||
type === 'repository' ||
type === 'project' ? (
<Milestones data={recentMilestones} showAvatar={showAvatar} />
) : (
<RecentReleases data={recentReleases} showAvatar={showAvatar} showSingleColumn={true} />
)}
</div>
)}
{(type === 'project' ||
type === 'repository' ||
type === 'organization' ||
type === 'user') && (
<div className="grid-cols-2 gap-4 lg:grid">
<RecentPullRequests data={pullRequests} showAvatar={showAvatar} />
<RecentReleases data={recentReleases} showAvatar={showAvatar} showSingleColumn={true} />
</div>
)}
Comment on lines +28 to +52
Copy link
Contributor

@coderabbitai coderabbitai bot Oct 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Fix redundant conditional logic and unreachable code.

The logic contains multiple critical issues:

  1. Redundant outer conditions: Lines 28-31 and 44-47 check identical type values ('project' || 'repository' || 'user' || 'organization'), so both blocks always render together or not at all.

  2. Unreachable else branch: Lines 34-37 check the same four types within the first outer block, making the condition always true. The else branch (lines 39-41) is dead code that can never execute.

This suggests the original intent was different conditional logic for different type combinations.

Clarify the intended rendering logic. If both grids should always display together for these types, simplify to:

-  <>
-    {(type === 'project' ||
-      type === 'repository' ||
-      type === 'user' ||
-      type === 'organization') && (
+  <>
+    {(type === 'project' || type === 'repository' || type === 'user' || type === 'organization') && (
+      <>
       <div className="grid-cols-2 gap-4 lg:grid">
         <RecentIssues data={recentIssues} showAvatar={showAvatar} />
-        {type === 'user' ||
-        type === 'organization' ||
-        type === 'repository' ||
-        type === 'project' ? (
-          <Milestones data={recentMilestones} showAvatar={showAvatar} />
-        ) : (
-          <RecentReleases data={recentReleases} showAvatar={showAvatar} showSingleColumn={true} />
-        )}
+        <Milestones data={recentMilestones} showAvatar={showAvatar} />
       </div>
-    )}
-    {(type === 'project' ||
-      type === 'repository' ||
-      type === 'organization' ||
-      type === 'user') && (
       <div className="grid-cols-2 gap-4 lg:grid">
         <RecentPullRequests data={pullRequests} showAvatar={showAvatar} />
         <RecentReleases data={recentReleases} showAvatar={showAvatar} showSingleColumn={true} />
       </div>
+      </>
     )}
   </>

If different types should show different content, please clarify the requirements.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
In frontend/src/components/ActivitySection.tsx around lines 28 to 52 there are
redundant identical outer conditions and an unreachable else branch; fix by
consolidating the logic: either (A) if the intent is to render both grids for
the same set of types, replace the duplicated outer checks with a single
conditional for (project|repository|user|organization) and render the two grid
divs (remove the inner else dead branch and always render Milestones in the
first grid), or (B) if the intent was conditional content per type, change the
inner condition to the correct subset (for example only user|organization should
render Milestones and the else should handle project|repository to render
RecentReleases) and keep one outer check that encloses the appropriate grids;
preserve existing component props like showSingleColumn when moving code.

</>
)

export default ActivitySection;
6 changes: 5 additions & 1 deletion frontend/src/components/AnchorTitle.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,11 @@ const AnchorTitle: React.FC<AnchorTitleProps> = ({ title }) => {
return (
<div id={id} className="relative">
<div className="group relative flex items-center">
<div className="flex items-center text-2xl font-semibold" data-anchor-title="true">
<div
className="flex items-center text-2xl font-semibold"
data-testid="anchor-title"
data-anchor-title="true"
>
{title}
</div>
<a
Expand Down
Loading