Skip to content

fix: improve keyboard shortcuts dialog scrolling behavior and cursor#522

Merged
mazeincoding merged 1 commit into
OpenCut-app:stagingfrom
karansingh21202:fix/keyboard-shortcuts-dialog-scroll
Aug 15, 2025
Merged

fix: improve keyboard shortcuts dialog scrolling behavior and cursor#522
mazeincoding merged 1 commit into
OpenCut-app:stagingfrom
karansingh21202:fix/keyboard-shortcuts-dialog-scroll

Conversation

@karansingh21202
Copy link
Copy Markdown
Contributor

@karansingh21202 karansingh21202 commented Aug 5, 2025

fix: improve keyboard shortcuts dialog scrolling and interaction

Problem:

  • The entire keyboard shortcuts dialog was scrolling, including the header and footer
  • The close button didn't show a pointer cursor, making it less obvious it was clickable
  • The layout had inconsistent spacing and padding

Solution:

  • Made only the content area scrollable while keeping the header and footer fixed
  • Added cursor-pointer to the close button for better UX
  • Improved overall dialog layout with consistent spacing and padding
  • Fixed line ending issues in the affected files

Testing:

  • Verified that the content scrolls independently while header/footer stay fixed
  • Confirmed the close button shows a pointer cursor on hover
  • Checked the layout on different screen sizes

Notes:

  • This improves the overall user experience when viewing and interacting with keyboard shortcuts

Summary by CodeRabbit

  • Style
    • Improved the layout and scroll behavior of the keyboard shortcuts help dialog for better usability and consistency.
    • Updated dialog appearance, including scrollable content areas and refined header, footer, and close button styling.

- Made only the content area scrollable while keeping header and footer fixed
- Added proper cursor pointer to dialog close button
- Improved overall dialog layout and spacing
@vercel
Copy link
Copy Markdown

vercel Bot commented Aug 5, 2025

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

A member of the Team first needs to authorize it.

@netlify
Copy link
Copy Markdown

netlify Bot commented Aug 5, 2025

👷 Deploy request for appcut pending review.

Visit the deploys page to approve it

Name Link
🔨 Latest commit a88d42f

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Aug 5, 2025

Walkthrough

This change restructures the layout and styling of the keyboard shortcuts help dialog and the dialog content container. It introduces a scrollable shortcuts list area, updates padding and flex behavior for header and footer sections, and simplifies the dialog content's internal structure. No logic or event handling is altered.

Changes

Cohort / File(s) Change Summary
Keyboard Shortcuts Help Dialog Layout
apps/web/src/components/keyboard-shortcuts-help.tsx
Refactored layout to use a flex column structure with max height, explicit header/footer padding, and a scrollable shortcuts list area. Adjusted button and container padding for improved scroll and layout control.
Dialog Content Structure
apps/web/src/components/ui/dialog.tsx
Removed internal ScrollArea and extra padding div from DialogContent, rendering children directly. Updated close button styling for cursor and positioning. No changes to exported/public interfaces.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant KeyboardShortcutsHelpDialog
    participant DialogContent

    User->>KeyboardShortcutsHelpDialog: Opens shortcuts help
    KeyboardShortcutsHelpDialog->>DialogContent: Renders dialog content
    DialogContent-->>KeyboardShortcutsHelpDialog: Provides scrollable, padded layout
    User->>KeyboardShortcutsHelpDialog: Interacts with shortcuts list or closes dialog
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~7 minutes

Possibly related PRs

Poem

In a dialog box, neat and tall,
The shortcuts now scroll, no more wall-to-wall.
Headers are padded, footers aligned,
The close button’s easy for users to find.
With layout refined, the UI feels bright—
A rabbit’s delight in code done right! 🐇✨

Note

⚡️ Unit Test Generation is now available in beta!

Learn more here, or try it out under "Finishing Touches" below.


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d022a78 and a88d42f.

📒 Files selected for processing (2)
  • apps/web/src/components/keyboard-shortcuts-help.tsx (2 hunks)
  • apps/web/src/components/ui/dialog.tsx (1 hunks)
🧰 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/components/keyboard-shortcuts-help.tsx
  • apps/web/src/components/ui/dialog.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/components/keyboard-shortcuts-help.tsx
  • apps/web/src/components/ui/dialog.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/components/keyboard-shortcuts-help.tsx
  • apps/web/src/components/ui/dialog.tsx
🧠 Learnings (8)
📚 Learning: applies to **/*.{jsx,tsx} : use semantic elements instead of role attributes in jsx....
Learnt from: CR
PR: OpenCut-app/OpenCut#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.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't assign interactive aria roles to non-interactive html elements....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Don't assign interactive ARIA roles to non-interactive HTML elements.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't add extra closing tags for components without children....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Don't add extra closing tags for components without children.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't set `aria-hidden="true"` on focusable elements....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't set `aria-hidden="true"` on focusable elements.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't assign non-interactive aria roles to interactive html elements....
Learnt from: CR
PR: OpenCut-app/OpenCut#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.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't pass children as props....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-07-27T22:14:46.402Z
Learning: Applies to **/*.{jsx,tsx} : Don't pass children as props.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't use both `children` and `dangerouslysetinnerhtml` props on the sam...
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't use both `children` and `dangerouslySetInnerHTML` props on the same element.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
📚 Learning: applies to **/*.{jsx,tsx} : don't use dangerous jsx props....
Learnt from: CR
PR: OpenCut-app/OpenCut#0
File: .cursor/rules/ultracite.mdc:0-0
Timestamp: 2025-07-27T22:15:27.748Z
Learning: Applies to **/*.{jsx,tsx} : Don't use dangerous JSX props.

Applied to files:

  • apps/web/src/components/ui/dialog.tsx
🔇 Additional comments (6)
apps/web/src/components/ui/dialog.tsx (2)

55-55: LGTM! Simplified children rendering improves flexibility.

The direct rendering of children without wrapper elements allows consuming components to have full control over their internal layout and scrolling behavior, which aligns with the PR's goal of fixing scrolling issues.


56-56: Good UX improvement with the cursor pointer.

Adding cursor-pointer to the close button makes it clearer that the element is interactive, addressing the UX concern mentioned in the PR objectives.

apps/web/src/components/keyboard-shortcuts-help.tsx (4)

202-202: Excellent layout restructuring for controlled scrolling.

The flex column layout with max-h-[80vh] and p-0 creates a proper container where individual sections can control their own padding and scrolling behavior. This directly addresses the PR objective of preventing the entire dialog from scrolling.


203-203: Good use of flex-shrink-0 for fixed header.

Making the header non-shrinkable with explicit padding ensures it remains visible and properly spaced even when content overflows. This maintains the header in a fixed position as intended.


214-238: Well-implemented scrollable content area.

The combination of overflow-y-auto, flex-grow, and scrollbar-thin creates a clean scrolling experience where only the shortcuts list scrolls while maintaining proper internal padding. This is exactly what was needed to fix the scrolling behavior issue.


239-239: Consistent footer implementation.

Using flex-shrink-0 with explicit padding matches the header approach and ensures the footer remains fixed at the bottom with consistent spacing. The removal of the button's top margin is appropriate since the footer now handles the spacing.

✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • 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
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@mazeincoding mazeincoding changed the base branch from main to staging August 15, 2025 00:49
@mazeincoding mazeincoding merged commit 32d263f into OpenCut-app:staging Aug 15, 2025
1 of 8 checks passed
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.

2 participants