From 7d9bb0d4fb975712d73db506d8c14a87b1d1b88d Mon Sep 17 00:00:00 2001 From: AaronDavidNewman Date: Sun, 10 Dec 2023 15:11:56 -0600 Subject: [PATCH 1/3] fix bug 1603 - key signature on non-treble stave, no glyph None of our test cases had clef with non-default clef (e.g. bass) and a key signature, without the clef symbol displayed. I added a test case and created a 'addClefLines' that adds the logical clef (e.g. the lines match the clef) but without the glyph. --- src/stave.ts | 9 ++++++++- tests/keysignature_tests.ts | 30 ++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/stave.ts b/src/stave.ts index 193f890a1b..478933f73d 100644 --- a/src/stave.ts +++ b/src/stave.ts @@ -443,7 +443,14 @@ export class Stave extends Element { } return this; } - + /** + * treat the stave as if the clef is clefSpec, but don't display the clef + * @param clefSpec + */ + setClefLines(clefSpec: string) { + this.clef = clefSpec; + return this; + } setClef(clefSpec: string, size?: string, annotation?: string, position?: number): this { if (position === undefined) { position = StaveModifierPosition.BEGIN; diff --git a/tests/keysignature_tests.ts b/tests/keysignature_tests.ts index 41bf1a7dae..fdfaca37f0 100644 --- a/tests/keysignature_tests.ts +++ b/tests/keysignature_tests.ts @@ -26,6 +26,7 @@ const KeySignatureTests = { run('Altered key test', majorKeysAltered); run('End key with clef test', endKeyWithClef); run('Key Signature Change test', changeKey); + run('Key Signature with/without clef symbol', clefKeySignature); }, }; @@ -370,5 +371,34 @@ function changeKey(options: TestOptions): void { options.assert.ok(true, 'all pass'); } +function clefKeySignature(options: TestOptions): void { + const f = VexFlowTests.makeFactory(options, 900); + + // The previous code was buggy: f.Stave(10, 10, 800), even though Factory.Stave() only accepts 1 argument. + const stave = f.Stave({ x: 10, y: 10, width: 800 }).addClef('bass').addTimeSignature('C|').setClefLines('bass'); + + const voice = f + .Voice() + .setStrict(false) + .addTickables([ + f.KeySigNote({ key: 'Bb' }), + f.StaveNote({ keys: ['c/4'], duration: '1' }), + f.BarNote(), + f.KeySigNote({ key: 'D', cancelKey: 'Bb' }), + f.StaveNote({ keys: ['c/4'], duration: '1' }), + f.BarNote(), + f.KeySigNote({ key: 'Bb' }), + f.StaveNote({ keys: ['c/4'], duration: '1' }), + f.BarNote(), + f.KeySigNote({ key: 'D', alterKey: ['b', 'n'] }), // TODO: alterKey needs to be a string[] + f.StaveNote({ keys: ['c/4'], duration: '1' }), + ]); + + f.Formatter().joinVoices([voice]).formatToStave([voice], stave); + + f.draw(); + + options.assert.ok(true, 'all pass'); +} VexFlowTests.register(KeySignatureTests); export { KeySignatureTests }; From b35952b79501fcf49d05a5e4f85d103a1489b95f Mon Sep 17 00:00:00 2001 From: AaronDavidNewman Date: Sun, 10 Dec 2023 15:21:36 -0600 Subject: [PATCH 2/3] fix the note in the test case to be middle C --- tests/keysignature_tests.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/keysignature_tests.ts b/tests/keysignature_tests.ts index fdfaca37f0..5add7624db 100644 --- a/tests/keysignature_tests.ts +++ b/tests/keysignature_tests.ts @@ -382,16 +382,16 @@ function clefKeySignature(options: TestOptions): void { .setStrict(false) .addTickables([ f.KeySigNote({ key: 'Bb' }), - f.StaveNote({ keys: ['c/4'], duration: '1' }), + f.StaveNote({ keys: ['c/4'], duration: '1', clef: 'bass' }), f.BarNote(), f.KeySigNote({ key: 'D', cancelKey: 'Bb' }), - f.StaveNote({ keys: ['c/4'], duration: '1' }), + f.StaveNote({ keys: ['c/4'], duration: '1', clef: 'bass' }), f.BarNote(), f.KeySigNote({ key: 'Bb' }), - f.StaveNote({ keys: ['c/4'], duration: '1' }), + f.StaveNote({ keys: ['c/4'], duration: '1', clef: 'bass' }), f.BarNote(), f.KeySigNote({ key: 'D', alterKey: ['b', 'n'] }), // TODO: alterKey needs to be a string[] - f.StaveNote({ keys: ['c/4'], duration: '1' }), + f.StaveNote({ keys: ['c/4'], duration: '1', clef: 'bass' }), ]); f.Formatter().joinVoices([voice]).formatToStave([voice], stave); From 770015fae3ac0f3a24f8cfa15a0a728058db2d94 Mon Sep 17 00:00:00 2001 From: AaronDavidNewman Date: Sat, 6 Jan 2024 11:39:23 -0600 Subject: [PATCH 3/3] code review comments --- src/stave.ts | 3 ++- tests/keysignature_tests.ts | 5 ++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/stave.ts b/src/stave.ts index 478933f73d..943366ef67 100644 --- a/src/stave.ts +++ b/src/stave.ts @@ -443,14 +443,15 @@ export class Stave extends Element { } return this; } + /** * treat the stave as if the clef is clefSpec, but don't display the clef - * @param clefSpec */ setClefLines(clefSpec: string) { this.clef = clefSpec; return this; } + setClef(clefSpec: string, size?: string, annotation?: string, position?: number): this { if (position === undefined) { position = StaveModifierPosition.BEGIN; diff --git a/tests/keysignature_tests.ts b/tests/keysignature_tests.ts index 5add7624db..3eba25eea9 100644 --- a/tests/keysignature_tests.ts +++ b/tests/keysignature_tests.ts @@ -373,8 +373,6 @@ function changeKey(options: TestOptions): void { function clefKeySignature(options: TestOptions): void { const f = VexFlowTests.makeFactory(options, 900); - - // The previous code was buggy: f.Stave(10, 10, 800), even though Factory.Stave() only accepts 1 argument. const stave = f.Stave({ x: 10, y: 10, width: 800 }).addClef('bass').addTimeSignature('C|').setClefLines('bass'); const voice = f @@ -390,7 +388,7 @@ function clefKeySignature(options: TestOptions): void { f.KeySigNote({ key: 'Bb' }), f.StaveNote({ keys: ['c/4'], duration: '1', clef: 'bass' }), f.BarNote(), - f.KeySigNote({ key: 'D', alterKey: ['b', 'n'] }), // TODO: alterKey needs to be a string[] + f.KeySigNote({ key: 'D', alterKey: ['b', 'n'] }), f.StaveNote({ keys: ['c/4'], duration: '1', clef: 'bass' }), ]); @@ -400,5 +398,6 @@ function clefKeySignature(options: TestOptions): void { options.assert.ok(true, 'all pass'); } + VexFlowTests.register(KeySignatureTests); export { KeySignatureTests };