Skip to content

feat(editor): expand snap grid options - #57

Merged
byrongamatos merged 1 commit into
feat/shortcut-interaction-settingsfrom
feat/editor-expanded-snap-grid
Jul 5, 2026
Merged

feat(editor): expand snap grid options#57
byrongamatos merged 1 commit into
feat/shortcut-interaction-settingsfrom
feat/editor-expanded-snap-grid

Conversation

@ChrisBeWithYou

Copy link
Copy Markdown
Contributor

Summary

  • expand editor snap options beyond 1/16 through 1/96
  • include triplet-friendly snap divisions such as 1/12, 1/24, and 1/48
  • make snap options data-driven while preserving the existing snap index model and 1/4 default
  • add snap option model tests for labels and beat subdivisions

Testing

  • node --check screen.js
  • node tests/snap_options.test.js
  • all tests/*.test.js via node

@coderabbitai

coderabbitai Bot commented Jul 4, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 168d853c-d866-4064-afa4-9ee8a8ae0790

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/editor-expanded-snap-grid

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

@byrongamatos

Copy link
Copy Markdown
Collaborator

Review pass (Claude Code) — stacked on #56. Own logic is correct: SNAP_OPTIONS (label/value/subdivisions) with SNAP_VALUES derived from it keeps the index model + 1/4 default (snapIdx: 2), _editorSnapSubdivisionsPure handles the 1/N→subdivisions math, and snapTime's if (!sv …) correctly treats the Off entry as no-snap. node --check + all JS tests green (3 snap-option tests).

⚠️ Conflict flag (not a bug here): this overlaps #46 (editor-snap-navigation), which also expands the snap grid but with an incompatible model — #46 removes SNAP_VALUES and adds a separate snapEnabled flag + triplet labels (1/3T, 1/12T), while this PR keeps SNAP_VALUES (derived) with an Off entry and plain 1/12/1/24 labels. Both redefine the same constants, so they can't both merge. Same family as the #46#55 nav conflict — worth deciding which snap model wins alongside items 2/3. (Codex rate-limited — manual + test-backed.)

@byrongamatos
byrongamatos force-pushed the feat/shortcut-interaction-settings branch from 37ded33 to 1d4a60c Compare July 5, 2026 12:24
@byrongamatos
byrongamatos force-pushed the feat/editor-expanded-snap-grid branch from cff0e97 to 93fa52f Compare July 5, 2026 12:24
@byrongamatos
byrongamatos merged commit 39cd58d into feat/shortcut-interaction-settings Jul 5, 2026
1 check passed
byrongamatos pushed a commit that referenced this pull request Jul 5, 2026
The snap grid jumped from 1/4 straight to 1/8 with no quarter- or
eighth-note triplet resolution, so triplet passages could only snap at
1/12T or finer. Add 1/3T (value 1/3) and 1/6T (value 1/6) in value order,
and label the triplet-family divisions (3/6/12/24/48/96) with a T suffix
so the grid reads clearly against the binary options.

The snap <select> options are static in screen.html and index-mapped via
editorSetSnap(selectedIndex), so screen.html, SNAP_OPTIONS, and the
snapIdx default are kept in lockstep; 1/4 shifts from index 2 to 3, so the
S.snapIdx default and snap_options.test.js move with it. snapIdx is not
persisted, so no migration is needed.

Ports the one division set #62 (feat/snap-grid-on-profile-stack) had over
the merged #57/#58 snap options, without adopting its {step} engine.

Tests: snap_options.test.js updated (+1/3, +1/6 subdivisions, new label
list, default index); full editor JS suite green (24 files).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UR2Cr7GEu3yMY7SrfxH6c1
byrongamatos pushed a commit that referenced this pull request Jul 5, 2026
The snap grid jumped from 1/4 straight to 1/8 with no quarter- or
eighth-note triplet resolution, so triplet passages could only snap at
1/12T or finer. Add 1/3T (value 1/3) and 1/6T (value 1/6) in value order,
and label the triplet-family divisions (3/6/12/24/48/96) with a T suffix
so the grid reads clearly against the binary options.

The snap <select> options are static in screen.html and index-mapped via
editorSetSnap(selectedIndex), so screen.html, SNAP_OPTIONS, and the
snapIdx default are kept in lockstep; 1/4 shifts from index 2 to 3, so the
S.snapIdx default and snap_options.test.js move with it. snapIdx is not
persisted, so no migration is needed.

Ports the one division set #62 (feat/snap-grid-on-profile-stack) had over
the merged #57/#58 snap options, without adopting its {step} engine.

Tests: snap_options.test.js updated (+1/3, +1/6 subdivisions, new label
list, default index); full editor JS suite green (24 files).


Claude-Session: https://claude.ai/code/session_01UR2Cr7GEu3yMY7SrfxH6c1

Co-authored-by: ChrisBeWithYou <chris@rifflarr.local>
Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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