Skip to content

Conversation

@nileshpahari
Copy link

Proposed change

Add type="button" attribute to all interactive buttons that are not meant to submit forms.

Resolves #2656

Files modified

  • frontend/src/app/page.tsx
  • frontend/src/app/projects/dashboard/metrics/page.tsx
  • frontend/src/components/MenteeIssues.tsx
  • frontend/src/components/Milestones.tsx
  • frontend/src/components/MultiSearch.tsx
  • frontend/src/components/NavDropDown.tsx
  • frontend/src/components/RecentIssues.tsx
  • frontend/src/components/RecentPullRequests.tsx
  • frontend/src/components/Release.tsx
  • frontend/src/components/RepositoryCard.tsx
  • frontend/src/components/ScrollToTop.tsx
  • frontend/src/components/Search.tsx
  • frontend/src/components/SortBy.tsx
  • frontend/src/components/ToggleableList.tsx
  • frontend/src/components/UserMenu.tsx

Checklist

  • I've read and followed the contributing guidelines.
  • I've run make check-test locally; all checks and tests passed.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Dec 3, 2025

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Updated button elements with explicit type attributes for consistent form handling behavior across the application.

✏️ Tip: You can customize this high-level summary in your review settings.

Walkthrough

Adds explicit type="button" attributes to button elements across multiple frontend files (including one test update) to prevent unintended form submissions; no other logic, rendering, or public API changes.

Changes

Cohort / File(s) Summary
App pages
frontend/src/app/page.tsx, frontend/src/app/projects/dashboard/metrics/page.tsx
Added type="button" to button elements
Components
frontend/src/components/*
frontend/src/components/MenteeIssues.tsx, frontend/src/components/Milestones.tsx, frontend/src/components/MultiSearch.tsx, frontend/src/components/NavDropDown.tsx, frontend/src/components/RecentIssues.tsx, frontend/src/components/RecentPullRequests.tsx, frontend/src/components/Release.tsx, frontend/src/components/RepositoryCard.tsx, frontend/src/components/ScrollToTop.tsx, frontend/src/components/Search.tsx, frontend/src/components/SortBy.tsx, frontend/src/components/ToggleableList.tsx, frontend/src/components/UserMenu.tsx
Added type="button" to various button elements to avoid default form submission; no other behavior changes
Auth / Login
frontend/src/components/LoginPageContent.tsx, frontend/__tests__/unit/components/LoginPageContent.test.tsx
Added type="button" to GitHub sign-in button and updated test expectation accordingly

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Homogeneous, low-complexity edits (attribute additions) across many files.
  • Pay extra attention to:
    • frontend/src/components/LoginPageContent.tsx and its updated test.
    • frontend/src/components/UserMenu.tsx where multiple buttons were changed.
    • Any buttons inside actual form elements to confirm intended types.

Possibly related PRs

Suggested labels

frontend-tests

Suggested reviewers

  • arkid15r
  • kasya

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly describes the main change: adding missing type attributes to buttons, which directly matches all code changes across multiple components.
Description check ✅ Passed The PR description is directly related to the changeset, stating the proposed change of adding type="button" attributes and listing all modified files that match the raw summary.
Linked Issues check ✅ Passed The PR successfully addresses issue #2656 by adding explicit type="button" attributes to all interactive buttons not meant to submit forms across 15 components, meeting the acceptance criteria.
Out of Scope Changes check ✅ Passed All changes are in-scope: they exclusively add type="button" attributes to buttons across components and update the corresponding test, with no unrelated modifications.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8810c2b and 9acbad2.

📒 Files selected for processing (2)
  • frontend/__tests__/unit/components/LoginPageContent.test.tsx (1 hunks)
  • frontend/src/components/LoginPageContent.tsx (1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/ProgramDetails.test.tsx:35-0
Timestamp: 2025-07-12T17:12:25.807Z
Learning: In React applications, button elements should always have an explicit type attribute (type="button", type="submit", or type="reset") to prevent unintended form submission behavior, even when the button appears to be outside of a form context. The default type is "submit" which can cause unexpected behavior.
📚 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/components/LoginPageContent.test.tsx
📚 Learning: 2025-07-12T17:12:25.807Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 1717
File: frontend/__tests__/unit/pages/ProgramDetails.test.tsx:35-0
Timestamp: 2025-07-12T17:12:25.807Z
Learning: In React applications, button elements should always have an explicit type attribute (type="button", type="submit", or type="reset") to prevent unintended form submission behavior, even when the button appears to be outside of a form context. The default type is "submit" which can cause unexpected behavior.

Applied to files:

  • frontend/__tests__/unit/components/LoginPageContent.test.tsx
  • frontend/src/components/LoginPageContent.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/components/LoginPageContent.test.tsx
📚 Learning: 2025-09-17T02:42:41.928Z
Learnt from: Rajgupta36
Repo: OWASP/Nest PR: 2288
File: frontend/src/components/ActionButton.tsx:0-0
Timestamp: 2025-09-17T02:42:41.928Z
Learning: In frontend/src/components/ActionButton.tsx, the user Rajgupta36 intentionally changed text-blue-600 to text-[#1D7BD7] to align the text color with the border color (#1D7BD7) for visual consistency, prioritizing design alignment over theme tokens.

Applied to files:

  • frontend/src/components/LoginPageContent.tsx
🔇 Additional comments (2)
frontend/src/components/LoginPageContent.tsx (1)

73-80: Explicit type="button" is correct and aligns with PR goal

Setting the GitHub sign‑in button’s type="button" is appropriate here (no form submission intent) and prevents future accidental submits if this gets wrapped in a <form>. This also matches the repo’s guidance to always specify button types.

frontend/__tests__/unit/components/LoginPageContent.test.tsx (1)

396-409: Test assertion correctly encodes the button-type requirement

The added assertion (with explanatory comment) that the sign‑in button has type="button" cleanly captures the intended behavior and will prevent regressions if the type is removed or changed. This is fully aligned with the linked issue’s acceptance criteria and prior learnings about explicit button types.

Tip

✨ Issue Enrichment is now available for GitHub issues!

CodeRabbit can now help you manage issues more effectively:

  • Duplicate Detection — Identify similar or duplicate issues
  • Related Issues & PRs — Find relevant issues and PR's from your repository
  • Suggested Assignees — Find the best person to work on the issue
  • Implementation Planning — Generate detailed coding plans for engineers and agents
Disable automatic issue enrichment

To disable automatic issue enrichment, add the following to your .coderabbit.yaml:

issue_enrichment:
  auto_enrich:
    enabled: false

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.

@nileshpahari
Copy link
Author

button in frontend/src/components/LoginPageContent.tsx is also missing type="button" but adding it is causing frontend/__tests__/unit/components/LoginPageContent.test.tsx to fail

@kasya
Copy link
Collaborator

kasya commented Dec 5, 2025

button in frontend/src/components/LoginPageContent.tsx is also missing type="button" but adding it is causing frontend/__tests__/unit/components/LoginPageContent.test.tsx to fail

@nileshpahari you should be able to edit the test to accommodate this change, making sure old functionality still present and works.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Dec 6, 2025

@nileshpahari
Copy link
Author

button in frontend/src/components/LoginPageContent.tsx is also missing type="button" but adding it is causing frontend/__tests__/unit/components/LoginPageContent.test.tsx to fail

@nileshpahari you should be able to edit the test to accommodate this change, making sure old functionality still present and works.

Done. Please check when you're free.

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.

Missing Type Attributes on Buttons

2 participants