Limit visible Roadmap & Timeline items on About page #3343
Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughLimits initial Roadmap items to 3 and Timeline items to 6 on the About page, adds Show more/Show less toggles with client-side state and a ShowMoreButton, expands mock milestone data, and updates unit and e2e tests to exercise the new show/hide behavior. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (1)📚 Learning: 2025-06-20T16:12:59.256ZApplied to files:
🧬 Code graph analysis (1)frontend/src/app/about/page.tsx (2)
⏰ 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). (9)
🔇 Additional comments (4)
✏️ Tip: You can disable this entire section by setting 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/app/about/page.tsx (1)
203-248: Roadmap filtering not accounted for in Show more logic.The slice (line 209) and button visibility condition (line 245) both use the unfiltered array length (
projectMetadata.recentMilestones.length), but the displayed milestones are filtered bystate !== 'closed'. If some milestones are closed, the button may appear when there are ≤3 visible items.🐛 Proposed fix to use filtered array length
{projectMetadata.recentMilestones.length > 0 && ( <SecondaryCard icon={FaMapSigns} title={<AnchorTitle title="Roadmap" />}> <div className="grid gap-4"> - {[...projectMetadata.recentMilestones] - .filter((milestone) => milestone.state !== 'closed') - .sort((a, b) => (a.title > b.title ? 1 : -1)) - .slice(0, showAllRoadmap ? projectMetadata.recentMilestones.length : 3) - .map((milestone, index) => ( + {(() => { + const filteredMilestones = [...projectMetadata.recentMilestones] + .filter((milestone) => milestone.state !== 'closed') + .sort((a, b) => (a.title > b.title ? 1 : -1)) + return filteredMilestones + .slice(0, showAllRoadmap ? filteredMilestones.length : 3) + .map((milestone, index) => ( <div key={milestone.url || milestone.title || index} ... </div> - ))} + )) + })()} </div> - {projectMetadata.recentMilestones.length > 3 && ( + {[...projectMetadata.recentMilestones].filter((m) => m.state !== 'closed').length > 3 && ( <ShowMoreButton onToggle={() => setShowAllRoadmap(!showAllRoadmap)} /> )} </SecondaryCard> )}Alternatively, compute
filteredMilestonesonce viauseMemoand reference it in both places for cleaner code.
🤖 Fix all issues with AI agents
In `@frontend/__tests__/unit/pages/About.test.tsx`:
- Around line 682-687: In the About.test.tsx unit test (About.test.tsx) there's
a duplicated assertion calling expect(screen.getByText('Project
Timeline')).toBeInTheDocument() twice; remove the redundant assertion (the
second occurrence) so the test only asserts the presence of "Project Timeline"
once and keep the remaining assertions for Timeline Event 7, Timeline Event 2,
and the negative check for Timeline Event 1.
In `@frontend/src/app/about/page.tsx`:
- Around line 261-279: The connector line condition compares index to
projectTimeline.length (the original array) causing the last visible item to
still render a connector when sliced; create a visible array (e.g.,
visibleTimeline = [...projectTimeline].reverse().slice(0, showAllTimeline ?
projectTimeline.length : 6)) and iterate over visibleTimeline instead, then
change the condition to use visibleTimeline.length (index !==
visibleTimeline.length - 1) so the connector is omitted for the actual last
rendered milestone; update the map call to visibleTimeline.map and keep keys and
JSX as-is.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/__tests__/mockData/mockAboutData.tsfrontend/__tests__/unit/pages/About.test.tsxfrontend/src/app/about/page.tsx
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-11-17T16:47:05.578Z
Learnt from: anurag2787
Repo: OWASP/Nest PR: 2671
File: frontend/__tests__/unit/components/MultiSearch.test.tsx:427-427
Timestamp: 2025-11-17T16:47:05.578Z
Learning: In the frontend test files for the OWASP/Nest repository, `expect(true).toBe(true)` no-op assertions may be intentionally added as workarounds when ESLint rule jest/expect-expect doesn't detect expectations inside helper functions or waitFor callbacks. These can be resolved by configuring the ESLint rule's assertFunctionNames option to recognize custom assertion helpers instead of flagging them as redundant.
Applied to files:
frontend/__tests__/unit/pages/About.test.tsx
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/unit/pages/About.test.tsx
🧬 Code graph analysis (2)
frontend/__tests__/unit/pages/About.test.tsx (1)
frontend/__tests__/mockData/mockAboutData.ts (1)
mockAboutData(1-62)
frontend/src/app/about/page.tsx (1)
frontend/src/utils/aboutData.ts (1)
projectTimeline(61-138)
🔇 Additional comments (7)
frontend/__tests__/mockData/mockAboutData.ts (1)
26-37: LGTM!Test data appropriately extended to support verifying the "Show more" functionality. The two additional milestones ensure the mock data exceeds the 3-item initial display limit for the Roadmap section.
frontend/src/app/about/page.tsx (2)
91-93: LGTM!State variables are correctly initialized and well-placed after the query hooks.
281-283: LGTM!The button visibility condition correctly uses the static timeline array length.
frontend/__tests__/unit/pages/About.test.tsx (4)
73-78: LGTM!Mock timeline data appropriately extended to 7 events, enabling verification of the 6-item initial limit and expand/collapse behavior.
167-202: LGTM!The ShowMoreButton mock properly maintains toggle state and renders appropriate button text, enabling accurate test assertions for expand/collapse interactions.
278-310: LGTM!Comprehensive test coverage for the timeline Show more/less functionality. Good use of scoped queries with
within()to target the correct section's button.
409-438: LGTM!Good test structure that replicates the implementation's sorting logic to derive expected visible items. Scoped queries ensure the correct ShowMoreButton is targeted.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
|
You have to adress coderabbit issues |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/app/about/page.tsx`:
- Around line 206-209: The chained filter/sort/slice uses
projectMetadata.recentMilestones.length for the slice when showAllRoadmap is
true but the array was filtered first; compute the filtered array once (e.g.,
const filteredMilestones = projectMetadata.recentMilestones.filter(m => m.state
!== 'closed').sort(...)) and then use filteredMilestones.slice(0, showAllRoadmap
? filteredMilestones.length : 3) in the JSX and update the later condition that
checks count (the one referencing the unfiltered count) to use
filteredMilestones.length so all logic consistently uses the filtered array; you
can create the variable above the JSX or wrap the rendering in an IIFE/timeline
pattern to keep scope local.
🧹 Nitpick comments (2)
frontend/__tests__/e2e/pages/About.spec.ts (1)
77-83: Section locator could be fragile.The locator chain
.filter({ has: heading }).filter({ has: button }).last()may match unintended elements if the page structure changes. Consider using a more specific selector like finding the heading and traversing to its parentSecondaryCardcomponent.However, if this pattern works reliably in your e2e environment and the team prefers it, it's acceptable.
frontend/__tests__/unit/pages/About.test.tsx (1)
283-293: Remove duplicate assertions.Lines 291-292 duplicate the assertions on lines 284 and 286. These redundant checks add noise without additional value.
Proposed fix
expect(screen.getByText('2029')).toBeInTheDocument() expect(screen.queryByText('2023')).not.toBeInTheDocument() expect(screen.getByText('2024')).toBeInTheDocument() - - expect(screen.getByText('Timeline Event 7')).toBeInTheDocument() - expect(screen.queryByText('Timeline Event 1')).not.toBeInTheDocument() const timelineSection = screen.getByText('Project Timeline').closest('h2')?.parentElement
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
frontend/__tests__/e2e/pages/About.spec.tsfrontend/__tests__/unit/pages/About.test.tsxfrontend/src/app/about/page.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-11-17T16:47:05.578Z
Learnt from: anurag2787
Repo: OWASP/Nest PR: 2671
File: frontend/__tests__/unit/components/MultiSearch.test.tsx:427-427
Timestamp: 2025-11-17T16:47:05.578Z
Learning: In the frontend test files for the OWASP/Nest repository, `expect(true).toBe(true)` no-op assertions may be intentionally added as workarounds when ESLint rule jest/expect-expect doesn't detect expectations inside helper functions or waitFor callbacks. These can be resolved by configuring the ESLint rule's assertFunctionNames option to recognize custom assertion helpers instead of flagging them as redundant.
Applied to files:
frontend/__tests__/unit/pages/About.test.tsxfrontend/__tests__/e2e/pages/About.spec.ts
📚 Learning: 2025-11-17T17:30:42.139Z
Learnt from: anurag2787
Repo: OWASP/Nest PR: 2671
File: frontend/__tests__/unit/components/MultiSearch.test.tsx:169-171
Timestamp: 2025-11-17T17:30:42.139Z
Learning: In the OWASP/Nest frontend tests (PR `#2671` context), wrapper functions like `expectChaptersCountEqualsThree` that simply call another helper with a fixed parameter (e.g., `expectChaptersCountEquals(3)`) are intentionally used to avoid arrow function callbacks in `waitFor` calls. This pattern prevents adding nesting depth that would trigger rule typescript:S2004. Example: `await waitFor(expectChaptersCountEqualsThree)` avoids the extra nesting from `await waitFor(() => expectChaptersCountEquals(3))`.
Applied to files:
frontend/__tests__/unit/pages/About.test.tsxfrontend/__tests__/e2e/pages/About.spec.ts
📚 Learning: 2025-07-12T17:36:57.255Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/createProgram.test.tsx:70-86
Timestamp: 2025-07-12T17:36:57.255Z
Learning: When testing React page components that use mocked form components, validation logic should be tested at the form component level, not the page level. Page-level tests should focus on authentication, role checking, submission handling, and navigation logic.
Applied to files:
frontend/__tests__/e2e/pages/About.spec.ts
🧬 Code graph analysis (3)
frontend/__tests__/unit/pages/About.test.tsx (1)
frontend/__tests__/mockData/mockAboutData.ts (1)
mockAboutData(1-62)
frontend/__tests__/e2e/pages/About.spec.ts (1)
frontend/__tests__/mockData/mockAboutData.ts (1)
mockAboutData(1-62)
frontend/src/app/about/page.tsx (1)
frontend/src/utils/aboutData.ts (1)
projectTimeline(61-138)
🔇 Additional comments (9)
frontend/src/app/about/page.tsx (2)
261-285: LGTM! Timeline implementation correctly addresses the connector line issue.The IIFE pattern properly computes
visibleTimelineand uses its length for the connector condition on line 267. This resolves the previously flagged issue where the connector would incorrectly render for the last visible item.
91-93: LGTM!State initialization for show/hide toggles is appropriate, defaulting to collapsed view as intended by the PR requirements.
frontend/__tests__/e2e/pages/About.spec.ts (2)
72-89: LGTM!The roadmap e2e test correctly verifies:
- Initial visible state (line 75)
- Show more button interaction (line 83)
- All milestones visible after expansion (lines 85-88)
Using
{ exact: true }for title matching is a good practice.
112-126: LGTM!The timeline e2e test correctly verifies:
- Most recent item ("OWASP Nest Logo Introduction") is visible initially
- Oldest item ("Project Inception") becomes visible after clicking Show more
This aligns with the reversed timeline display (newest first) and the 6-item initial limit.
frontend/__tests__/unit/pages/About.test.tsx (5)
167-202: LGTM!The
ShowMoreButtonmock appropriately maintains internal state to toggle between "Show more" and "Show less" labels, which is necessary for the tests to simulate real user interactions.
278-310: LGTM!The timeline test correctly verifies:
- Initial state with 6 visible items (Event 7 visible, Event 1 hidden)
- Expansion via Show more reveals Event 1
- Collapse via Show less hides Event 1 again
Using
within()to scope button queries to the timeline section is good practice.
409-438: LGTM!The roadmap test correctly:
- Mirrors the component's sort logic (lines 416-418)
- Verifies the initial 3-item limit
- Tests Show more/Show less toggle behavior
This ensures the test accurately reflects the component's behavior.
353-366: LGTM!Good use of
within()to scope the Show more/Show less button queries to the Wall of Fame section. This prevents ambiguity now that multiple sections have these buttons.
678-686: LGTM!This simpler timeline test verifies the initial collapsed state without interaction. The assertions correctly reflect that Timeline Event 7 (newest) is visible while Timeline Event 1 (oldest) is hidden with the 6-item default limit.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@frontend/src/app/about/page.tsx`:
- Around line 250-251: ShowMoreButton's internal isExpanded can desync from the
parent boolean (showAllRoadmap/showAllTimeline); make it controlled by adding an
isExpanded prop to ShowMoreButton and use that value for rendering instead of
its own internal state, then pass the parent state (e.g., showAllRoadmap) from
the parent where ShowMoreButton is used and keep the existing onToggle to flip
parent state, or if you prefer to keep internal state add a useEffect inside
ShowMoreButton that updates its internal isExpanded whenever a new prop (e.g.,
expanded or controlledIsExpanded) changes so the component stays in sync with
the parent's showAllRoadmap/showAllTimeline.
🧹 Nitpick comments (1)
frontend/src/app/about/page.tsx (1)
205-254: Well-structured IIFE pattern addressing the previous review feedback.The filtered milestones array is now correctly scoped, and both the slice and the ShowMoreButton condition properly use
filteredMilestones.length.One minor note: the sort comparator on line 208 doesn't handle equal titles (returns
-1instead of0), which can cause unstable sorting. This is unlikely to matter if milestone titles are unique, but for completeness:♻️ Optional: Stable sort comparator
const filteredMilestones = [...projectMetadata.recentMilestones] .filter((milestone) => milestone.state !== 'closed') - .sort((a, b) => (a.title > b.title ? 1 : -1)) + .sort((a, b) => a.title.localeCompare(b.title))
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/src/app/about/page.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-06-20T16:12:59.256Z
Learnt from: ahmedxgouda
Repo: OWASP/Nest PR: 1633
File: frontend/src/components/HealthMetrics.tsx:30-30
Timestamp: 2025-06-20T16:12:59.256Z
Learning: In the DetailsCard component (frontend/src/components/CardDetailsPage.tsx), there's a length check before rendering HealthMetrics: `healthMetricsData.length > 0`. This ensures that when HealthMetrics is rendered, the data array has at least one element, making accessing data[0] safe within the HealthMetrics component.
Applied to files:
frontend/src/app/about/page.tsx
🔇 Additional comments (3)
frontend/src/app/about/page.tsx (3)
8-8: LGTM!Import changes are appropriate—
useStateis needed for the new toggle states, andShowMoreButtonis correctly imported for use in both sections.Also applies to: 31-31
91-93: LGTM!State hooks are properly initialized with
falseto show limited items by default, matching the PR requirements (3 for Roadmap, 6 for Timeline).
268-292: LGTM! Previous connector line issue is correctly resolved.The IIFE pattern properly scopes
visibleTimeline, and the connector line condition at line 274 now usesvisibleTimeline.length - 1, ensuring no dangling connector appears on the last visible item when collapsed.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
frontend/src/app/about/page.tsx
Outdated
| </div> | ||
| ))} | ||
| </div> | ||
| {filteredMilestones.length > 3 && ( |
frontend/src/app/about/page.tsx
Outdated
| )) | ||
| })()} | ||
| </div> | ||
| {projectTimeline.length > 6 && ( |
|
kasya
left a comment
kasya
left a comment
There was a problem hiding this comment.
@HarshitVerma109 this works great! Thank you 👍🏼
4 participants
Proposed change
Resolves #3057
Implemented "Show more" functionality for the Roadmap and Project Timeline sections on the About page.
Checklist
make check-testlocally: all warnings addressed, tests passed