Skip to content

Comments

fix:remove unused react props#3204

Merged
kasya merged 12 commits intoOWASP:mainfrom
Nikhilkumarr-dev:fix/remove-unused-props
Jan 13, 2026
Merged

fix:remove unused react props#3204
kasya merged 12 commits intoOWASP:mainfrom
Nikhilkumarr-dev:fix/remove-unused-props

Conversation

@Nikhilkumarr-dev
Copy link
Contributor

@Nikhilkumarr-dev Nikhilkumarr-dev commented Jan 5, 2026

Proposed change

Removed unused props from MockTooltipProps in test file frontend/tests/unit/components/Card.test.tsx,This helps keep the codebase clean, improves performance, and enhances code readability.

Resolves #3139

Checklist

  • Required: I read and followed the contributing guidelines
  • Required: I ran make check-test locally and all tests passed
  • I used AI for code, documentation, or tests in this PR

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 5, 2026

Walkthrough

Two unused optional properties (closeDelay and delay) were removed from the MockTooltipProps interface in a test file, simplifying the mock tooltip prop definition without affecting any component logic or test behavior.

Changes

Cohort / File(s) Summary
Test Mock Interface Cleanup
frontend/__tests__/unit/components/Card.test.tsx
Removed unused optional properties closeDelay?: number and delay?: number from the MockTooltipProps interface

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~2 minutes

Possibly related PRs

Suggested labels

frontend, frontend-tests

Suggested reviewers

  • arkid15r
  • kasya
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Linked Issues check ⚠️ Warning The PR removes closeDelay and delay props as required by issue #3139, but does not remove the showArrow prop which was explicitly listed as needing removal. Remove the unused showArrow prop from MockTooltipProps to fully address all requirements in issue #3139.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: removing unused React props from the codebase, which aligns with the changeset.
Out of Scope Changes check ✅ Passed All changes are scoped to the linked issue #3139, focusing on removing unused props from the test file with no extraneous modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Description check ✅ Passed The pull request description clearly explains the change: removal of unused props from MockTooltipProps in the test file to improve code cleanliness and readability, with a reference to issue #3139.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Nikhilkumarr-dev
Copy link
Contributor Author

hey @arkid15r hope this issue is resolved,Could you please review and let me know if you’d like any changes or improvements?
Thank you!

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

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

@Nikhilkumarr-dev Hi! Please resolve conflicts ⬇️
Also, it's required to run make check-test locally as it says on the template.
Screenshot 2026-01-07 at 6 55 02 PM

@Nikhilkumarr-dev
Copy link
Contributor Author

hey @kasya ,Let me know if any changes need to be made.
Thank You.

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

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

@Nikhilkumarr-dev Hi! Yes, there are still some SOnar issues to address. Plus tests are failing.

Make sure you run make check-test locally before every push to make sure all tests pass AND all checks complete.

Image Image

@Nikhilkumarr-dev Nikhilkumarr-dev force-pushed the fix/remove-unused-props branch 2 times, most recently from ef9e589 to 8948815 Compare January 10, 2026 14:23
@Nikhilkumarr-dev
Copy link
Contributor Author

hey @kasya,The sonar cloud errors we are seeing are below
image

but our main branch is ahead of this we are using heroui instead of @fortawesome/react-fontawesome this component library
Screenshot 2026-01-10 195754

The main branch is currently ahead, and the issue continues to appear in SonarCloud despite removing the unused React props. I would appreciate your feedback on this specific matter—hope I’ve explained it clearly.
Thank You.

@kasya
Copy link
Collaborator

kasya commented Jan 10, 2026

hey @kasya,The sonar cloud errors we are seeing are below image

but our main branch is ahead of this we are using heroui instead of @fortawesome/react-fontawesome this component library Screenshot 2026-01-10 195754

The main branch is currently ahead, and the issue continues to appear in SonarCloud despite removing the unused React props. I would appreciate your feedback on this specific matter—hope I’ve explained it clearly. Thank You.

@Nikhilkumarr-dev DId you run tests locally with these changes?
They fail right now due to this change:
Screenshot 2026-01-10 at 11 56 48 AM

Please aways verify there are no breaking changes with running tests locally.

Also, these Sonar errors are in your branch - they have nothing to do with main state.

Copy link
Collaborator

@kasya kasya left a comment

Choose a reason for hiding this comment

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

@Nikhilkumarr-dev You haven't fixed the tests 🤔
They are still failing to compile.

Image

You also haven't ran make check-test locally before commit - it is required and we can tell when you're not doing it.

@Nikhilkumarr-dev
Copy link
Contributor Author

Nikhilkumarr-dev commented Jan 12, 2026

Hey @kasya I ran the make check-test ,I haven't seen any error before committing.
I will work on it again

@Nikhilkumarr-dev
Copy link
Contributor Author

Hello .
While pre-commit, test ending up successfully without any test errors previously and now.
After commiting to the feature branch and making pr facing with this issue below in img ,already working on it and reached up to community noted some imp points.
Screenshot 2026-01-12 211218
Screenshot 2026-01-12 211153
@kasya Hope u help me out in figuring out this.
Thank you

@kasya
Copy link
Collaborator

kasya commented Jan 13, 2026

Hello . While pre-commit, test ending up successfully without any test errors previously and now. After commiting to the feature branch and making pr facing with this issue below in img ,already working on it and reached up to community noted some imp points.

hi @Nikhilkumarr-dev !
The main issue you have is that you're using WSL. We do not support that, but I believe there are other contributors who either made it work somehow or found another way to run project.

So when you run make check-test your version fails on running checks and you never actually get to running tests, and tests are what was actually failing.
You could have ran make test-frontend-unit to run tests only and fix them. You can find all of our Make commands in Makefile either global one, or frontend/backend related.

So, I did fix the tests for you - the problem was with icons mock. Since we switched to react-icons mocks should be different now and there were examples in the code of how that should be done.

I'll push my changes and will merge this in, but I recommend you join our channel in Slack and maybe reach out to other Nest contributors to help with the project setup to be sure you can follow our workflow fully. 👌🏼

@sonarqubecloud
Copy link

@Nikhilkumarr-dev
Copy link
Contributor Author

Gm @kasya Absolutely very helpfull really u made a valid point.Thank You for your kind help and suggestion.
I will switch to Ubuntu and make sure to setup and start contributing..
Thank you

@kasya kasya added this pull request to the merge queue Jan 13, 2026
Merged via the queue into OWASP:main with commit a50e87d Jan 13, 2026
27 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Remove Unused react typed Props

2 participants