feat: forward cents from custom pitch blocks and make always-visible#7770
feat: forward cents from custom pitch blocks and make always-visible#7770021nirav-blip wants to merge 3 commits into
Conversation
github-actions
Bot
added
feature
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #7770 +/- ##
=======================================
Coverage 55.99% 56.00%
=======================================
Files 172 172
Lines 57397 57407 +10
=======================================
+ Hits 32141 32151 +10
Misses 25256 25256 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
021nirav-blip
requested review from
Ashutoshx7,
omsuneri,
pikurasa and
walterbender
as code owners
|
@walterbender , @pikurasa |
| global.NOTESFLAT = ["C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B"]; | ||
| global.NOTESTEP = { C: 1, D: 3, E: 5, F: 6, G: 8, A: 10, B: 12 }; | ||
| global.EQUIVALENTNATURALS = { | ||
| "E\u266F": "F", |
There was a problem hiding this comment.
We have constants defined for the various accidentals. We should use those as it makes the code easier to read.
|
|
||
| describe("CustomNoteBlock", () => { | ||
| const customNoteCents = [ | ||
| ["C(+42¢)", "C", 42], |
There was a problem hiding this comment.
We should prob. define a constant for the ¢ symbol too. (I don't think there is one.)
|
What is the best way to test this inside of MB? C+0¢ And when you click on it, it opens the solfege menu with no ability to adjust cents. Maybe not relevant to this PR, but perhaps we should (1) open a pie menu with letter names instead and (2) allow some adjustment of cents? Then it really would be a custom note? Once a temperament is defined, it would by default live in that range of notes -- but still, being able to tweak cents might be a good addition. |
- Add _parseCents() static helper to CustomNoteBlock - CustomNoteBlock.flow() parses (+N¢)/(-N¢) suffix and forwards cents - CustomPitchBlock.flow() parses and forwards cents instead of hardcoded 0 - Set hidden=false for both blocks (always visible in palette) - Add tests for _parseCents, cents forwarding, and always-visible Related to sugarlabs#7171
Walter review feedback: - Define CENTSSYMBOL constant in musicutils.js and export it - Use SHARP/FLAT constants instead of unicode literals in tests - Use CENTSSYMBOL constant instead of literal ¢ in test data Related to sugarlabs#7171
When changing a note via the pie menu, preserve any existing cents suffix (e.g., C(+50¢) → D(+50¢) instead of just D). Related to sugarlabs#7171
021nirav-blip
force-pushed
the
refactor-custom-pitch-blocks
branch
from
c6012a4 to
e618c9a
Compare
2 participants
this PR finally makes custom pitch blocks actually respect cents and also makes them findable.
What's new:
Cents now forward. CustomNoteBlock.flow() and CustomPitchBlock.flow() parse "C(+42¢)" : ["C", 42] and send the cents to the synth. Before, you could type cents in the UI but they'd just get ignored - you'd hear C+0¢.
Blocks are always visible. Both custom blocks now show up in the Pitch palette without needing a custom temperament first. They used to be hidden, which made them impossible to discover.
Pie menu preserves cents. If you have C(+50¢) and pick D from the pie menu, you get D(+50¢) - not just D. That was devins feedback, and it's fixed.
CENTSSYMBOL constant. Added "\u00A2" to musicutils.js and exported it. Tests now use SHARP/FLAT/CENTSSYMBOL constants instead of raw unicode - cleaner and more maintainable.
Quick test: drop a CustomNoteBlock, type C(+42¢), play – you should hear 42 cents above C. Pick D from the pie menu – cents stay. Both blocks are visible in the palette immediately.
Tests pass (6541/6541 – the 3 widget failures were already there).
Files touched: PitchBlocks.js, musicutils.js, piemenus.js.
Part of #7171.
PR Category