Skip to content

Allow to get width without stave#246

Merged
rvilarl merged 2 commits intovexflow:mainfrom
rvilarl:issue245
Sep 27, 2024
Merged

Allow to get width without stave#246
rvilarl merged 2 commits intovexflow:mainfrom
rvilarl:issue245

Conversation

@rvilarl
Copy link
Collaborator

@rvilarl rvilarl commented Sep 21, 2024

@rvilarl rvilarl requested a review from ronyeh September 21, 2024 08:23
Copy link
Member

@ronyeh ronyeh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will a keysignature object be formatted multiple times even though it doesn't have a stave? In that situation, we are returning multiple dummy Stave objects. Is it better if the format() method creates a new dummy Stave and then call this.setStave()?

Should we log some warning in that case? Otherwise the key signature is attached to a stave that the developer never created, and it's kind of a silent failure.

@rvilarl
Copy link
Collaborator Author

rvilarl commented Sep 24, 2024

@ronyeh thanks! Now setStave is used with a dummy Stave and commented. I prefer the comment because it is legitimate to use format without Stave to figure out the width.

@rvilarl rvilarl requested a review from ronyeh September 24, 2024 05:58
@ronyeh
Copy link
Member

ronyeh commented Sep 24, 2024 via email

@rvilarl
Copy link
Collaborator Author

rvilarl commented Sep 24, 2024

You are right I normally eslint. I will change it this evening. I will also remove the call to getStave.

@rvilarl
Copy link
Collaborator Author

rvilarl commented Sep 25, 2024

@ronyeh done

@rvilarl rvilarl merged commit a9d8a1c into vexflow:main Sep 27, 2024
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.

KeySignature allow to get width without assigned Stave

2 participants