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

Add more event support #231

Merged
merged 22 commits into from
Jun 24, 2024
Merged

Add more event support #231

merged 22 commits into from
Jun 24, 2024

Conversation

jaredjj3
Copy link
Collaborator

@jaredjj3 jaredjj3 commented Jun 20, 2024

This PR fixes #227 by adding a basic event system. It adds an overlay element, which makes the event behavior consistent between SVG and canvas. Without this, SVG elements would "steal" touchend and touchcancel. I decided to leave the event system very bare and simple for now. I'll try to revisit before 1.0.0.

@jaredjj3 jaredjj3 self-assigned this Jun 20, 2024
@jaredjj3
Copy link
Collaborator Author

It offers a slight advantage when an accidental is rendered, but the same could be achieve with boxes. Take a look at how the circle is off-center for notes with accidentals:
image

Vexflow includes the accidentals, flags and other modifiers in the bounding box. Of course bounding boxes are sometimes too big. See third note.

image

@rvilarl let's continue the conversation from #230 (comment) here.

I'm not getting the same result as the test. I'm using vexflow.StaveNote.getBoundingBox.

image

In the tests, I saw something dealing with ModifierContext that I'm not doing in vexml explicitly. I logged vexflow.StaveNote.getModifierContext and got this:

image

I suspect that this ModifierContext is incorrect. What's the correct way to use this so that the bounding box includes the modifiers?

@rvilarl
Copy link
Collaborator

rvilarl commented Jun 21, 2024

To me the ModifierContext looks good: there is one Accidental and one StaveNote as expected.
What version of VexFlow are you using now? Are you adding the Modifiers to the notes (with addModifier)?

@jaredjj3
Copy link
Collaborator Author

I'm using vxflw-early-access 5.0.0-alpha.11 and I'm using vexflow.StaveNote.addModifier (code).

@jaredjj3 jaredjj3 marked this pull request as ready for review June 24, 2024 02:23
@jaredjj3 jaredjj3 merged commit 2712364 into master Jun 24, 2024
1 check passed
@jaredjj3 jaredjj3 deleted the events branch June 24, 2024 02:23
@rvilarl
Copy link
Collaborator

rvilarl commented Jun 24, 2024

I'm using vxflw-early-access 5.0.0-alpha.11 and I'm using vexflow.StaveNote.addModifier (code).

@jaredjj3 please try with vexflow 5.0.0-alpha.4 we do not release now in vxflw-early-access.

@jaredjj3 jaredjj3 mentioned this pull request Jun 24, 2024
@rvilarl
Copy link
Collaborator

rvilarl commented Jun 25, 2024

@jaredjj3 this is why I said that I had been working in BoundigBox. I resolved some problems but I expect more bugs. Let me know if you come across them.

@jaredjj3
Copy link
Collaborator Author

@rvilarl will do, thanks for improving it!

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