Skip to content

Conversation

@sagarkawad
Copy link

@sagarkawad sagarkawad commented Jan 2, 2026

Description

With this fix, when you go to projects page (http://localhost:3000/projects) and click on "Select Projects" button, you no longer get the error - cannot contain a nested .
This bug occured as the checkboxes were rendered as buttons, which were nested inside actual buttons. To fix this issue the actual buttons were changed to divs with the assigned role of a button.

Fixes #675 (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)

How Has This Been Tested?

Manual testing was performed by going to /projects route and clicking on the "Select Projects" button to check if the functionality or any other features don't break.

  • Clicking on "Select Projects" button and checking if the card content is correctly shown and also other buttons are working fine.

Test Configuration:

  • Node version: 18.x
  • Browser (if applicable): Chrome (Latest)
  • Operating System: macOS

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have added screenshots if ui has been changed
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • Improvements
    • Enhanced keyboard accessibility for project selection controls, enabling seamless keyboard navigation and activation throughout the projects interface. Project cards can now be selected and toggled using keyboard input, improving usability for all users navigating the application.

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

@netlify
Copy link

netlify bot commented Jan 2, 2026

👷 Deploy request for appcut pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit ff7f16e

@vercel
Copy link

vercel bot commented Jan 2, 2026

@sagarkawad is attempting to deploy a commit to the OpenCut OSS Team on Vercel.

A member of the Team first needs to authorize it.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Jan 2, 2026

📝 Walkthrough

Walkthrough

The pull request replaces button elements with semantically accessible div elements (role="button") in the projects page to resolve HTML validation errors caused by nested buttons. Event handlers and interaction logic remain unchanged.

Changes

Cohort / File(s) Summary
Projects page selection controls
apps/web/src/app/projects/page.tsx
Converted button elements to div-based controls with role="button" and tabIndex attributes. Preserved onClick and onKeyDown event handlers and selection toggle functionality in the ProjectCard component and selection-enabled card wrapper.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 Nested buttons caused quite a fuss,
So we swapped them for divs—no more a big cuss!
With role and tabIndex in place,
Accessibility wins the race! 🎯

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 title clearly and specifically describes the bug being fixed: nested button elements causing a DOM validation error.
Description check ✅ Passed The description covers the main sections including the bug summary, type of change, testing approach, and most checklist items are completed.
Linked Issues check ✅ Passed The PR successfully addresses issue #675 by replacing nested button elements with divs using role='button', eliminating the ' cannot contain a nested ' error.
Out of Scope Changes check ✅ Passed All changes in the PR are directly related to fixing the nested button issue in the projects page without introducing unrelated modifications.
✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0173db9 and ff7f16e.

📒 Files selected for processing (1)
  • apps/web/src/app/projects/page.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{jsx,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{jsx,tsx}: Don't use accessKey attribute on any HTML element.
Don't set aria-hidden="true" on focusable elements.
Don't add ARIA roles, states, and properties to elements that don't support them.
Don't use distracting elements like <marquee> or <blink>.
Only use the scope prop on <th> elements.
Don't assign non-interactive ARIA roles to interactive HTML elements.
Make sure label elements have text content and are associated with an input.
Don't assign interactive ARIA roles to non-interactive HTML elements.
Don't assign tabIndex to non-interactive HTML elements.
Don't use positive integers for tabIndex property.
Don't include "image", "picture", or "photo" in img alt prop.
Don't use explicit role property that's the same as the implicit/default role.
Make static elements with click handlers use a valid role attribute.
Always include a title element for SVG elements.
Give all elements requiring alt text meaningful information for screen readers.
Make sure anchors have content that's accessible to screen readers.
Assign tabIndex to non-interactive HTML elements with aria-activedescendant.
Include all required ARIA attributes for elements with ARIA roles.
Make sure ARIA properties are valid for the element's supported roles.
Always include a type attribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden with aria-hidden).
Always include a lang attribute on the html element.
Always include a title attribute for iframe elements.
Accompany onClick with at least one of: onKeyUp, onKeyDown, or onKeyPress.
Accompany onMouseOver/onMouseOut with onFocus/onBlur.
Include caption tracks for audio and video elements.
Use semantic elements instead of role attributes in JSX.
Make sure all anchors are valid and navigable.
Ensure all ARIA properties (aria-*) are valid.
Use valid, non-abstract ARIA roles for elements with...

Files:

  • apps/web/src/app/projects/page.tsx
**/*.{js,jsx,ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{js,jsx,ts,tsx}: Don't use consecutive spaces in regular expression literals.
Don't use the arguments object.
Don't use the comma operator.
Don't write functions that exceed a given Cognitive Complexity score.
Don't use unnecessary boolean casts.
Don't use unnecessary callbacks with flatMap.
Use for...of statements instead of Array.forEach.
Don't create classes that only have static members (like a static namespace).
Don't use this and super in static contexts.
Don't use unnecessary catch clauses.
Don't use unnecessary constructors.
Don't use unnecessary continue statements.
Don't export empty modules that don't change anything.
Don't use unnecessary escape sequences in regular expression literals.
Don't use unnecessary labels.
Don't use unnecessary nested block statements.
Don't rename imports, exports, and destructured assignments to the same name.
Don't use unnecessary string or template literal concatenation.
Don't use String.raw in template literals when there are no escape sequences.
Don't use useless case statements in switch statements.
Don't use ternary operators when simpler alternatives exist.
Don't use useless this aliasing.
Don't initialize variables to undefined.
Don't use the void operators (they're not familiar).
Use arrow functions instead of function expressions.
Use Date.now() to get milliseconds since the Unix Epoch.
Use .flatMap() instead of map().flat() when possible.
Use literal property access instead of computed property access.
Don't use parseInt() or Number.parseInt() when binary, octal, or hexadecimal literals work.
Use concise optional chaining instead of chained logical expressions.
Use regular expression literals instead of the RegExp constructor when possible.
Don't use number literal object member names that aren't base 10 or use underscore separators.
Remove redundant terms from logical expressions.
Use while loops instead of for loops when you don't need initializer and update expressions.
Don't reassign const variables....

Files:

  • apps/web/src/app/projects/page.tsx
**/*.{ts,tsx}

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

**/*.{ts,tsx}: Don't use primitive type aliases or misleading types.
Don't use empty type parameters in type aliases and interfaces.
Don't use any or unknown as type constraints.
Don't return a value from a function with the return type 'void'.
Don't use the TypeScript directive @ts-ignore.
Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the ! postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Use as const instead of literal types and type annotations.
Use either T[] or Array<T> consistently.
Initialize each enum member value explicitly.
Use export type for types.
Use import type for types.
Make sure all enum members are literal values.
Don't use TypeScript const enum.
Don't declare empty interfaces.
Don't let variables evolve into any type through reassignments.
Don't use the any type.
Don't misuse the non-null assertion operator (!) in TypeScript files.
Don't use implicit any type on variable declarations.
Don't merge interfaces and classes unsafely.
Don't use overload signatures that aren't next to each other.
Use the namespace keyword instead of the module keyword to declare TypeScript namespaces.
Use consistent accessibility modifiers on class properties and methods.
Use function types instead of object types with call signatures.
Don't use void type outside of generic or return types.

**/*.{ts,tsx}: Don't use TypeScript enums.
Don't export imported variables.
Don't add type annotations to variables, parameters, and class properties that are initialized with literal expressions.
Don't use TypeScript namespaces.
Don't use non-null assertions with the ! postfix operator.
Don't use parameter properties in class constructors.
Don't use user-defined types.
Use as const instead of literal types and...

Files:

  • apps/web/src/app/projects/page.tsx
🧠 Learnings (6)
📓 Common learnings
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Make static elements with click handlers use a valid role attribute.
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Make static elements with click handlers use a valid role attribute.
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Use semantic elements instead of role attributes in JSX.
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Use semantic elements instead of role attributes in JSX.
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Use valid, non-abstract ARIA roles for elements with ARIA roles.
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Use valid, non-abstract ARIA roles for elements with ARIA roles.
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign non-interactive ARIA roles to interactive HTML elements.
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign non-interactive ARIA roles to interactive HTML elements.
📚 Learning: 2025-07-27T22:14:46.402Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Make static elements with click handlers use a valid role attribute.

Applied to files:

  • apps/web/src/app/projects/page.tsx
📚 Learning: 2025-07-27T22:15:27.748Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign interactive ARIA roles to non-interactive HTML elements.

Applied to files:

  • apps/web/src/app/projects/page.tsx
📚 Learning: 2025-07-27T22:15:27.748Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign non-interactive ARIA roles to interactive HTML elements.

Applied to files:

  • apps/web/src/app/projects/page.tsx
📚 Learning: 2025-07-27T22:15:27.748Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Use semantic elements instead of role attributes in JSX.

Applied to files:

  • apps/web/src/app/projects/page.tsx
📚 Learning: 2025-07-27T22:14:46.402Z
Learnt from: CR
Repo: OpenCut-app/OpenCut PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Make elements with interactive roles and handlers focusable.

Applied to files:

  • apps/web/src/app/projects/page.tsx
🔇 Additional comments (2)
apps/web/src/app/projects/page.tsx (2)

299-318: LGTM! Accessible implementation that correctly avoids nested buttons.

The div with role="button" provides proper keyboard accessibility with tabIndex={0} and handles both click and keyboard events (Enter/Space keys). Since the Checkbox inside (line 311) is used for display only (no onCheckedChange handler), the wrapper's interaction handlers control the selection logic without conflicts.

Using role="button" instead of a semantic <button> element is appropriate here to avoid nested button validation errors that would occur if the Checkbox component renders interactive elements internally.

Based on learnings: this follows the guideline to "make static elements with click handlers use a valid role attribute."


569-577: LGTM! Proper accessible wrapper that avoids nested buttons.

The div with role="button", tabIndex={0}, and keyboard handlers (Enter/Space keys) provides full accessibility for keyboard and screen reader users. The implementation correctly avoids nested buttons since the Checkbox component inside (lines 454-467) likely renders interactive elements, and the dropdown menu buttons (lines 494-553) are hidden when selection mode is active.

The Checkbox's onClick with stopPropagation() (line 462) properly prevents event bubbling to the wrapper, allowing both the card wrapper and the checkbox to be independently interactive without conflicts.

Based on learnings: this follows the guideline to "make static elements with click handlers use a valid role attribute" and "make elements with interactive roles and handlers focusable."


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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] <button> cannot contain a nested <button> at local dev

1 participant