fix:[BUG] <button> cannot contain a nested <button> by sagarkawad ยท Pull Request #678 ยท OpenCut-app/OpenCut

๐Ÿ“ 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.