Skip to content

feat(editor): add string move shortcuts - #84

Merged
byrongamatos merged 1 commit into
mainfrom
feat/editor-string-move-shortcuts
Jul 6, 2026
Merged

byrongamatos merged 1 commit into
mainfrom
feat/editor-string-move-shortcuts

Conversation

@ChrisBeWithYou

@ChrisBeWithYou ChrisBeWithYou commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

Summary

  • add plain Up/Down string-move commands to both shortcut profiles
  • move selected notes to adjacent strings without changing fret
  • keep Shift+Up/Down as the existing pitch-preserving string move behavior

Validation

  • node --check screen.js
  • node tests\eof_shortcuts.test.js
  • node tests\tempo_map_timesig.test.js
  • node tests\tempo_sync_inspector.test.js
  • node tests\tempo_map_bpm.test.js
  • node tests\tempo_map_guidance.test.js
  • node tests\measure_readout.test.js
  • git diff --cached --check
  • staged public-term guardrail scan

Summary by CodeRabbit

  • New Features
    • Added new keyboard shortcuts to move selected guitar/bass notes up or down one string while keeping the same fret position.
    • Plain ArrowUp and ArrowDown now trigger these string-move actions in supported editing modes.
  • Bug Fixes
    • Improved shortcut handling so string-move commands are only applied when a valid target string is available.
    • Updated shortcut mappings to better align the displayed key labels with the actual behavior.

@coderabbitai

coderabbitai Bot commented Jul 6, 2026

Copy link
Copy Markdown

Review Change Stack

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 642a96b1-6fc0-4d9f-a0c2-779f6c721983

📥 Commits

Reviewing files that changed from the base of the PR and between fc3e208 and f065c52.

📒 Files selected for processing (2)
  • screen.js
  • tests/eof_shortcuts.test.js

📝 Walkthrough

Walkthrough

Adds new moveStringUp/moveStringDown shortcut commands to screen.js that move selected notes to an adjacent string while preserving fret value, using new helper functions guarded against Keys mode and invalid targets. Commands are bound to ArrowUp/ArrowDown in EOF and FeedBack key dispatch, with corresponding test assertions added.

Changes

Move-to-string same-fret shortcuts

Layer / File(s) Summary
Core move-string logic and command routing
screen.js
Adds _getMoveStringSameFretResult and _execMoveStringSameFret helpers to move selected notes to an adjacent string with the same fret; registers moveStringUp/moveStringDown shortcut commands bound to ArrowUp/ArrowDown; wires dispatch in EOF and FeedBack key handling and command routing.
Shortcut mapping test coverage
tests/eof_shortcuts.test.js
Adds assertions that eof and feedback shortcut rows include the new commands, and that ArrowUp/ArrowDown (and shift+ArrowDown) map correctly in both profiles.

Estimated code review effort: 2 (Simple) | ~10 minutes

Possibly related PRs

  • got-feedBack/feedBack-plugin-editor#75: Both PRs modify screen.js's EOF/FeedBack key-to-command dispatch logic and extend tests/eof_shortcuts.test.js with new keyboard shortcut assertions.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/editor-string-move-shortcuts

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ast-grep (0.44.1)
screen.js

ast-grep timed out on this file


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

@byrongamatos
byrongamatos force-pushed the feat/editor-string-move-shortcuts branch from aae2dbd to dd3dcea Compare July 6, 2026 08:58
@byrongamatos
byrongamatos force-pushed the feat/editor-direct-fret-entry branch from 956be7b to 0ee0ba4 Compare July 6, 2026 09:16
@byrongamatos
byrongamatos force-pushed the feat/editor-string-move-shortcuts branch from dd3dcea to 842295c Compare July 6, 2026 09:16
@byrongamatos
byrongamatos changed the base branch from feat/editor-direct-fret-entry to main July 6, 2026 09:16
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