Skip to content

[Bugfix] constrain prevent default to not input fields#7

Merged
hlahtoo merged 2 commits intomainfrom
navigation-changes
Sep 18, 2025
Merged

[Bugfix] constrain prevent default to not input fields#7
hlahtoo merged 2 commits intomainfrom
navigation-changes

Conversation

@auchtopus
Copy link
Copy Markdown

@auchtopus auchtopus commented Sep 18, 2025

Description

Issue number and link

Testing plan

Author checklist

  • I have submitted a Contributor License Agreement
  • I have added my name to CONTRIBUTORS.md
  • I have updated CHANGES.md with a short summary of my change
  • I have added or updated unit tests to ensure consistent code coverage
  • I have updated the inline documentation, and included code examples where relevant
  • I have performed a self-review of my code

Summary by CodeRabbit

  • Bug Fixes
    • Corrected Space key handling: when focus is on form controls (input, textarea, select), pressing Space now behaves normally (e.g., inserts a space) instead of blocking default behavior.
    • Preserves previous behavior outside form fields, where Space continues to prevent page scrolling as before.
    • Improves typing and interaction within forms without affecting non-form keyboard interactions.

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented Sep 18, 2025

Caution

Review failed

The pull request is closed.

Walkthrough

Updated Space key handling in ScreenSpaceEventHandler to conditionally call event.preventDefault(). Space no longer blocks default behavior when focused on INPUT, TEXTAREA, or SELECT elements; previous prevention behavior remains for other targets. No public API signatures changed.

Changes

Cohort / File(s) Summary
Keyboard event handling
packages/engine/Source/Core/ScreenSpaceEventHandler.js
Modified handleKeyDown: Space key now calls preventDefault() only when event.target is not INPUT, TEXTAREA, or SELECT. Maintains prior behavior elsewhere; enables normal Space behavior in form controls.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant U as User
  participant B as Browser
  participant H as ScreenSpaceEventHandler

  U->>B: Press "Space"
  B->>H: keydown event (key=" ", target=element)

  rect rgba(200, 230, 255, 0.4)
  Note over H: Conditional preventDefault based on target tag
  alt Target is INPUT/TEXTAREA/SELECT
    H-->>B: Do not call preventDefault()
    Note over B: Default behavior continues (typing/scrolling as applicable)
  else Other elements
    H->>B: event.preventDefault()
    Note over B: Page scrolling prevented
  end
  end
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

I tapped the Space with gentle care,
Forms now breathe without a scare—
Inputs sing, textareas gleam,
Selects float in a scrolly dream.
Hop hop! says the rabbit dev,
Precise as keys in clover’s web.

✨ Finishing touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch navigation-changes

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

  • Linear integration is disabled by default for public repositories

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 3620386 and 0bc2e8c.

📒 Files selected for processing (1)
  • packages/engine/Source/Core/ScreenSpaceEventHandler.js (1 hunks)

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link
Copy Markdown

🔴 There was an error checking the CLA! If this is your first contribution, please send in a Contributor License Agreement.

    TypeError: key must be a string, a buffer or an object

@hlahtoo hlahtoo merged commit 6b89d90 into main Sep 18, 2025
0 of 5 checks passed
@hlahtoo hlahtoo deleted the navigation-changes branch September 18, 2025 21:49
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.

2 participants