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
- feat: makes the select all project button entirely clickable and cleans up small css changes #288: Modifies the same selection UI in
apps/web/src/app/projects/page.tsxto convert selection controls to clickable divs with adjusted styling and interaction patterns.
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
๐ 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 useaccessKeyattribute on any HTML element.
Don't setaria-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 thescopeprop 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 assigntabIndexto non-interactive HTML elements.
Don't use positive integers fortabIndexproperty.
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 atitleelement 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.
AssigntabIndexto non-interactive HTML elements witharia-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 atypeattribute for button elements.
Make elements with interactive roles and handlers focusable.
Give heading elements content that's accessible to screen readers (not hidden witharia-hidden).
Always include alangattribute on the html element.
Always include atitleattribute for iframe elements.
AccompanyonClickwith at least one of:onKeyUp,onKeyDown, oronKeyPress.
AccompanyonMouseOver/onMouseOutwithonFocus/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 theargumentsobject.
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 uselessthisaliasing.
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.
Useas constinstead of literal types and type annotations.
Use eitherT[]orArray<T>consistently.
Initialize each enum member value explicitly.
Useexport typefor types.
Useimport typefor 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.
Useas constinstead 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 withtabIndex={0}and handles both click and keyboard events (Enter/Space keys). Since the Checkbox inside (line 311) is used for display only (noonCheckedChangehandler), 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
onClickwithstopPropagation()(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.
Comment @coderabbitai help to get the list of available commands and usage tips.