Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update to vexflow beta #233

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

rvilarl
Copy link
Collaborator

@rvilarl rvilarl commented Sep 16, 2024

@jaredjj3 please help me fixing the error.

  [RuntimeError] NoStave: No stave attached to instance.

      77 |   @util.memoize()
      78 |   getWidth(): number {
    > 79 |     return this.getVfKeySignature().getWidth() + KEY_SIGNATURE_PADDING;

We need to set the stave before calling getWidth but I do not know how to get access to the stave in keysignature.ts

/** Returns the width of the key signature. */
  @util.memoize()
  getWidth(): number {
    return this.getVfKeySignature().getWidth() + KEY_SIGNATURE_PADDING;
  }

@jaredjj3
Copy link
Collaborator

I'm on mobile, apologies for the lack of formatting. Hopefully this unblocks you.

Right now, vexflow.Stave objects are created downstream in rendering.Part.getStaves. Some possible options:

  • Create a temporary vexflow.Stave solely for measuring width.
  • Create vexflow.Stave objects in rendering.StaveSignature and pass it accordingly to rendering.KeySignature.

@rvilarl
Copy link
Collaborator Author

rvilarl commented Sep 16, 2024

Thanks! I am considering if it makes sense to have the stave. This is required to calculate the bounding box position correctly but not the height and width.

@jaredjj3
Copy link
Collaborator

In keysignature.ts, this seemed to fix it:

  private getVfKeySignature(): vexflow.KeySignature {
    const vfStave = new vexflow.Stave(0, 0, 0);
    const vfKeySignature = new vexflow.KeySignature(
      this.getKey(),
      this.previousKeySignature?.getKey() ?? undefined,
      this.getAlterations()
    ).setPosition(vexflow.StaveModifierPosition.BEGIN);
    vfKeySignature.addToStave(vfStave);
    return vfKeySignature;
  }

However, I've been having issues running the test via yarn test (Docker) and yarn jest. The test setup probably needs to be updated.

@jaredjj3
Copy link
Collaborator

I was able to run the tests. The change I mentioned does produce diffs, but they're negligible.

@rvilarl
Copy link
Collaborator Author

rvilarl commented Sep 23, 2024

I have set a PR in vexflow to avoid changes here.it should be allowed to get the width without assigning a Stave, I think.

@jaredjj3
Copy link
Collaborator

Thanks! I think that's a better solution since it will prevent other vexflow users from breaking.

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