Conversation
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR removes the AnimatedCounter component entirely and replaces all its usages with static millified numeric values. Associated test files are removed or updated to reflect the transition from animated counters to static project statistics display. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ 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). (4)
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: 1
🧹 Nitpick comments (2)
frontend/src/app/page.tsx (1)
21-21: Static millified counters look fine, but confirm this is the desired resolution for #2868Switching the bottom stats to
{millify(stat.value)}+is a straightforward replacement for the previous AnimatedCounter usage and should render readable values.However, this fully removes the animation behavior rather than fixing it, which is a functional change relative to the original issue request (animate from 0 to target on scroll). Please confirm with maintainers that resolving #2868 by removing animation (instead of repairing it) is acceptable for UX/product goals.
If this is the intended long-term behavior, you may want to document the decision in the issue or PR description so future contributors don’t try to reintroduce animations based on the old requirement.
Also applies to: 376-380
frontend/src/app/about/page.tsx (1)
17-17: Clarify and simplify the millify rounding logic for project statsUsing
millify(Math.floor(stat.value / 10 || 0) * 10)will floor to the nearest lower multiple of 10 before millifying, which seems intentional to avoid noisy small changes. Thestat.value / 10 || 0piece, however, is a bit opaque and relies on JS truthiness.You could make this clearer and safer for nullish values by writing something like:
const roundedValue = Math.floor((stat.value ?? 0) / 10) * 10 const display = `${millify(roundedValue)}+`and then rendering
{display}. This preserves behavior but is easier to read and reason about.Please also confirm that snapping to the nearest 10 is the intended UX (as opposed to showing the exact count via millify); if not, you can drop the divide/floor/multiply and just use
millify(stat.value ?? 0) + '+'.Also applies to: 292-303
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
frontend/__tests__/e2e/pages/About.spec.ts(1 hunks)frontend/__tests__/unit/components/AnimatedCounter.test.tsx(0 hunks)frontend/__tests__/unit/pages/Home.test.tsx(1 hunks)frontend/src/app/about/page.tsx(2 hunks)frontend/src/app/page.tsx(2 hunks)frontend/src/components/AnimatedCounter.tsx(0 hunks)
💤 Files with no reviewable changes (2)
- frontend/tests/unit/components/AnimatedCounter.test.tsx
- frontend/src/components/AnimatedCounter.tsx
🧰 Additional context used
🧠 Learnings (3)
📚 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__/e2e/pages/About.spec.tsfrontend/__tests__/unit/pages/Home.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/Home.test.tsx
📚 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/Home.test.tsx
🔇 Additional comments (1)
frontend/__tests__/e2e/pages/About.spec.ts (1)
80-85: Test name now correctly reflects non-animated statsRenaming the test to "displays project statistics with correct values" matches the new static-display behavior without changing assertions. This keeps intent clear without affecting coverage.
|
* Refactored: removed animated counter * updated code * Update tests --------- Co-authored-by: Arkadii Yakovets <arkadii.yakovets@owasp.org>
2 participants
Proposed change
Resolves #2868
I've removed the usage of animated counter and kept statistics display as text.
While removing the usage of AnimatedCounter.tsx component, I had to update a couple of tests of home page and about page.
Checklist
make check-testlocally and all tests passed