0xfe / vexflow

A JavaScript library for rendering music notation and guitar tablature.
http://www.vexflow.com
Other
3.9k stars 662 forks source link

Input events #371

Closed harrislapiroff closed 2 years ago

harrislapiroff commented 8 years ago

Is there any plan to support events (such as hover, click, and tap events) within VexFlow? It could be useful for building music composition apps or music theory games!

(Also, VexFlow is incredible. Really amazing work. 👍)

gristow commented 8 years ago

This is already possible, if you're using SVGContext:

var elem = ctx.openGroup(className, idName);
crescendo.draw();
ctx.closeGroup();
elem.addEventListener("click", function(){
  console.log("Clickity clack.");
});

There are some limitations:

So, the solution I use is to:

Others may have different (better?) approaches.

You can see a running version of this I've done at http://bit.ly/note-draw

If you want to implement a somewhat full-fledged editor function, you'll probably need to go this route (since editing involves catching events on things that have not yet been created). If you just want a user to be able to, say, click on the correct note, the built-in group functionality should suffice.

harrislapiroff commented 8 years ago

Thanks for the info! Very useful example.

mscuthbert commented 8 years ago

Could we leave this issue open either (1) changing it to a Documentation feature. @gristow 's info is too important not to preserve elsewhere (and it'll otherwise just keep coming up). @harrislapiroff -- could you change the title to "Document user interaction with Vexflow" or something like that? OR (2) use it as a feature request to make it so that Gristow's last approach (solution) is easier for most people to implement?

For catching pointer events on a staff (such as adding and moving notes), I use a similar method to @gristow though I have been attaching the pointer events to a div (w/o border or padding) surrounding the SVG/Canvas (I have been using Canvas, but will switch to SVG soon). See music21j's stream.js addEditableCanvas() routine for more details. It is definitely possible, but I think it's within the scope of Vexflow to make this easier.

harrislapiroff commented 8 years ago

I could definitely see it being useful for VexFlow to facilitate/abstract out the logic in @gristow or your solution.

0xfe commented 8 years ago

VexFlow already has some elements instrumented to accept events, e.g., StaveNote. You can directly get the element with note.getElem(), or you can access them via the DOM using the vf-stavenote class. You can also decide if you want to include the stem or modifiers/accidentals in your events via the vf-stem or vf-modifiers classes.

I will happily accept PRs to instrument more VexFlow elements like this. It's quite straightforward -- the hardest part is coming up with decent class names.

0xfe commented 8 years ago

Also, yes, in the least documenting this (and @gristow's bounding box trick) is a good idea. :-)

gristow commented 8 years ago

A few thoughts: The hardest part of implementing this "bounding box trick" is converting co-ordinates in client/UI event space to those used within VexFlow. (This is relatively trivial if you're using a fixed-width svg or canvas; I typically have mine scaling responsively with the browser width.)

Would it make sense to have an addEventListener method on the various context classes which would:

Or at a minimum, we might add a clientCoordinatesToVexCoordinates method. (Users would still have to figure out how to go about implementing the tricky SVG overlay on their own in this case...)

As I remember (and I wrote the code for this about 13 months ago), the gotchas I encountered, that others might appreciate knowing about before trying this at home, were:

Getting all of this right was easily several days work for me, so it makes me think there might be some use in including this within VexFlow. It's not all that many lines of code. (On the other hand, I'm a musician, not a DOM expert -- this might be a breeze for others.)

0xfe commented 8 years ago

Actually, have you tried pointer-events: bounding-box on the element? Seems like it takes care of everything (I think): https://www.w3.org/TR/SVG2/interact.html#PointerEventsProp

0xfe commented 8 years ago

Looks like I added support for pointer-events: bounding-box to openGroup at some point. It think it worked at the time, but can't remember.

stavenote.js:

ctx.openGroup('flag', null, { pointerBBox: true });
Glyph.renderGlyph(ctx, flagX, flagY, glyph_font_scale, flagCode);
ctx.closeGroup();
mscuthbert commented 8 years ago

I believe that we've dropped support for IE <= 10, right? That's the last major browser not to support pointer-events. This would've been the reason not to publicize this in the past (Thanks @gristow for the difference between Canvas and SVG on what captures the events)

mscuthbert commented 8 years ago

Oh wait I was looking at the CSS/HTML support table; there doesn't seem to be a support table for SVG. (However, turning off pointer-events on the element might work to make it so that they fire just on the containing parent DIV)

gristow commented 7 years ago

This is an old thread -- but I've streamlined how I handle VexFlow user interaction for the exercises at uTheory.com. I have a generalized method I think is good which we might consider bringing into the VexFlow codebase.

In my case, I do this by extending the SVGContext class directly (we use a slightly customized version of VexFlow). But a generalized version of this, which SVGContext & CanvasContext could extend would be very easy to do.

Do we think this is of use? Here's the full code of how I do it:

/* global document window */
import SVGContext from './SVG';

const isDragSymbol = Symbol('dragging');
const isOutSymbol = Symbol('isOut');
const windowListeners = Symbol('windowListeners');

/**
 * A class to enable touch & mouse interactions on SVGs. Translates browser coordinates to
 * SVG coordinates.
 *
 * Call SVGInteraction.makeInteractive(svg), then override this class's methods to use.
 */
export default class SVGInteraction extends SVGContext {
  /* eslint-disable no-unused-vars */
  // These are here as holders -- override whichever you need when you inherit this class.
  touchStart(e, coords) {}
  touchEnd(e, coords) {}
  drag(e, coords) {}
  hover(e, coords) {}
  mouseOut(e, coords) {}
  /* eslint-enable no-unused-vars */

  makeInteractive(svg = this.svg) {
    // We will add listeners to the SVG bounding box itself:
    svg.style.pointerEvents = 'bounding-box';
    // An SVG point is used to translate div space to SVG space. See Alexander Jank's solution at:
    // https://stackoverflow.com/questions/29261304/how-to-get-the-click-coordinates-relative-to-svg-element-holding-the-onclick-lis
    this.svgPt = svg.createSVGPoint();

    /** ==== Condition Testing & Setting Functions ====
     *
     * These functions are used to translate UI events from the browser into more helpful
     * events from a programatic perspective. They take care of tracking mousestate &
     * touchstate (down or up) to fire drag vs. hover. If a user drags outside of the SVG,
     * they continue to fire the drag event, so that (if desired) things like drag & drop
     * among elements can be implemented.
     *
     */

    // A  not-combinator
    const not = condition => () => !condition();
    // Get whether we're dragging
    const isDragging = () => this[isDragSymbol];
    // Set whether the mouse is down or up.
    const down = () => { this[isDragSymbol] = true; return true; };
    const up = () => { this[isDragSymbol] = false; return true; };

    // Get whether we're outside of the SVG bounds
    const isOutside = () => this[isOutSymbol];
    // Set whether we're in our out; if we move out while dragging we need window listeners briefly.
    const inside = () => {
      this[isOutSymbol] = false;
      this[windowListeners].forEach(([eventType, listener]) => {
        window.removeEventListener(eventType, listener, false);
      });
      return true;
    };
    const outside = () => {
      this[isOutSymbol] = true;
      this[windowListeners].forEach(([eventType, listener]) => {
        window.addEventListener(eventType, listener);
      });
      return true;
    };

    // We'll hold the window listeners here so we can add & remove them as needed.
    this[windowListeners] = [];

    // Utility function to check event conditions & fire class events if needed
    const addListener = ([eventType, callback, ifTrue, el]) => {
      el = el || svg;
      const listener = (evt) => {
        if (ifTrue()) {
          const coords = getCoords(evt, svg, this.svgPt, this);
          callback.call(this, evt, coords);
        }
      };

      if (el !== window) el.addEventListener(eventType, listener);
      else this[windowListeners].push([eventType, listener]);
    };

      // [ type, listener, testFunction (fire listener if true), EventTarget (default this.svg)]
    [
      /* events occuring within SVG */
      ['mousedown', this.touchStart, down], // Touch is started
      ['touchstart', this.touchStart, down], // Touch is started
      ['mouseup', this.touchEnd, up], // Touch ends or mouse up
      ['touchend', this.touchEnd, up], // Touch ends or mouse up
      ['touchmove', this.drag, isDragging], // Dragging
      ['mousemove', this.drag, isDragging], // Dragging
      ['mousemove', this.hover, not(isDragging)], // Hover
      ['mouseout', this.mouseOut, not(isDragging)], // Mouseout
      ['touchcancel', this.mouseOut, not(isDragging)], // Mouseout (touch interrupt)
      ['mouseout', () => {}, () => isDragging() && outside()], // goes out of bounds isDown & set
      ['touchcancel', () => {}, () => isDragging() && outside()], // goes out of bounds isDown & set
      ['touchmove', inside, isOutside], // comes back inside
      ['mousemove', inside, isOutside], // comes back inside
      ['mousedown', inside, isOutside], // comes back inside, this shouldn't happen, but just in case
      ['touchstart', inside, isOutside], // comes back inside, this shouldn't happen, but just in case
      /* out of bounds events */
      ['touchmove', this.drag, () => isOutside() && isDragging(), window], // if outside in window & dragging
      ['mousemove', this.drag, () => isOutside() && isDragging(), window], // if outside in window & dragging
      ['mouseup', this.touchEnd, () => isOutside() && isDragging() && up() && inside(), window], // if outside in window & dragging & released
      ['touchend', this.touchEnd, () => isOutside() && isDragging() && up() && inside(), window], // if outside in window & dragging & released
    ]
    .forEach(addListener);
  }
}

/**
 * Returns coordinates for an event
 * @param {Event} e touch or mouse event
 * @param {SVGPoint} svgPt an SVG point
 * @param {SVGElement} svg the SVG's client bounding rectangle
 *
 * @returns {Object} { x, y, [touches] }
 */
function getCoords(e, svg, svgPt) {
  if ('touches' in e) {
    const touches = e.touches.map(touch => getCoords(touch, svg, svgPt));
    return { x: touches[0].x, y: touches[0].y, touches };
  }

  svgPt.x = e.clientX;
  svgPt.y = e.clientY;
  const svgCoords = svgPt.matrixTransform(svg.getScreenCTM().inverse());
  return { x: svgCoords.x, y: svgCoords.y };
}
0xfe commented 7 years ago

This looks pretty good, Gregory. Mind setting up a quick jsfiddle with an example to play with?

On Fri, Jun 9, 2017 at 1:01 PM, Gregory Ristow notifications@github.com wrote:

This is an old thread -- but I've streamlined how I handle VexFlow user interaction for the exercises at uTheory.com https://utheory.com. I have a generalized method I think is good which we might consider bringing into the VexFlow codebase.

In my case, I do this by extending the SVGContext class directly (we use a slightly customized version of VexFlow). But a generalized version of this, which SVGContext & CanvasContext could extend would be very easy to do.

Do we think this is of use? Here's the full code of how I do it:

/ global document window /import SVGContext from './SVG'; const isDragSymbol = Symbol('dragging');const isOutSymbol = Symbol('isOut');const windowListeners = Symbol('windowListeners'); /* A class to enable touch & mouse interactions on SVGs. Translates browser coordinates to SVG coordinates. Call SVGInteraction.makeInteractive(svg), then override this class's methods to use. /export default class SVGInteraction extends SVGContext { / eslint-disable no-unused-vars / // These are here as holders -- override whichever you need when you inherit this class. touchStart(e, coords) {} touchEnd(e, coords) {} drag(e, coords) {} hover(e, coords) {} mouseOut(e, coords) {} / eslint-enable no-unused-vars /

makeInteractive(svg = this.svg) { // We will add listeners to the SVG bounding box itself: svg.style.pointerEvents = 'bounding-box'; // An SVG point is used to translate div space to SVG space. See Alexander Jank's solution at: // https://stackoverflow.com/questions/29261304/how-to-get-the-click-coordinates-relative-to-svg-element-holding-the-onclick-lis this.svgPt = svg.createSVGPoint();

/** ==== Condition Testing & Setting Functions ====     *     * These functions are used to translate UI events from the browser into more helpful     * events from a programatic perspective. They take care of tracking mousestate &     * touchstate (down or up) to fire drag vs. hover. If a user drags outside of the SVG,     * they continue to fire the drag event, so that (if desired) things like drag & drop     * among elements can be implemented.     *     */

// A  not-combinator
const not = condition => () => !condition();
// Get whether we're dragging
const isDragging = () => this[isDragSymbol];
// Set whether the mouse is down or up.
const down = () => { this[isDragSymbol] = true; return true; };
const up = () => { this[isDragSymbol] = false; return true; };

// Get whether we're outside of the SVG bounds
const isOutside = () => this[isOutSymbol];
// Set whether we're in our out; if we move out while dragging we need window listeners briefly.
const inside = () => {
  this[isOutSymbol] = false;
  this[windowListeners].forEach(([eventType, listener]) => {
    window.removeEventListener(eventType, listener, false);
  });
  return true;
};
const outside = () => {
  this[isOutSymbol] = true;
  this[windowListeners].forEach(([eventType, listener]) => {
    window.addEventListener(eventType, listener);
  });
  return true;
};

// We'll hold the window listeners here so we can add & remove them as needed.
this[windowListeners] = [];

// Utility function to check event conditions & fire class events if needed
const addListener = ([eventType, callback, ifTrue, el]) => {
  el = el || svg;
  const listener = (evt) => {
    if (ifTrue()) {
      const coords = getCoords(evt, svg, this.svgPt, this);
      callback.call(this, evt, coords);
    }
  };

  if (el !== window) el.addEventListener(eventType, listener);
  else this[windowListeners].push([eventType, listener]);
};

  // [ type, listener, testFunction (fire listener if true), EventTarget (default this.svg)]
[
  /* events occuring within SVG */
  ['mousedown', this.touchStart, down], // Touch is started
  ['touchstart', this.touchStart, down], // Touch is started
  ['mouseup', this.touchEnd, up], // Touch ends or mouse up
  ['touchend', this.touchEnd, up], // Touch ends or mouse up
  ['touchmove', this.drag, isDragging], // Dragging
  ['mousemove', this.drag, isDragging], // Dragging
  ['mousemove', this.hover, not(isDragging)], // Hover
  ['mouseout', this.mouseOut, not(isDragging)], // Mouseout
  ['touchcancel', this.mouseOut, not(isDragging)], // Mouseout (touch interrupt)
  ['mouseout', () => {}, () => isDragging() && outside()], // goes out of bounds isDown & set
  ['touchcancel', () => {}, () => isDragging() && outside()], // goes out of bounds isDown & set
  ['touchmove', inside, isOutside], // comes back inside
  ['mousemove', inside, isOutside], // comes back inside
  ['mousedown', inside, isOutside], // comes back inside, this shouldn't happen, but just in case
  ['touchstart', inside, isOutside], // comes back inside, this shouldn't happen, but just in case
  /* out of bounds events */
  ['touchmove', this.drag, () => isOutside() && isDragging(), window], // if outside in window & dragging
  ['mousemove', this.drag, () => isOutside() && isDragging(), window], // if outside in window & dragging
  ['mouseup', this.touchEnd, () => isOutside() && isDragging() && up() && inside(), window], // if outside in window & dragging & released
  ['touchend', this.touchEnd, () => isOutside() && isDragging() && up() && inside(), window], // if outside in window & dragging & released
]
.forEach(addListener);

} } /* Returns coordinates for an event @param {Event} e touch or mouse event @param {number} height the internal SVG height @param {SVGPoint} svgPt an SVG point @param {SVGElement} svg the SVG's client bounding rectangle */function getCoords(e, svg, svgPt) { if ('touches' in e) { const touches = e.touches.map(touch => getCoords(touch, svg, svgPt)); return { x: touches[0].x, y: touches[0].y, touches }; }

svgPt.x = e.clientX; svgPt.y = e.clientY; const svgCoords = svgPt.matrixTransform(svg.getScreenCTM().inverse()); return { x: svgCoords.x, y: svgCoords.y }; }

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/0xfe/vexflow/issues/371#issuecomment-307444191, or mute the thread https://github.com/notifications/unsubscribe-auth/AAOukzSzY9j9s2y7f2ymEJOrpx64uiomks5sCXpygaJpZM4JAh7M .

-- Mohit Muthanna [mohit (at) muthanna (uhuh) com]

gristow commented 7 years ago

Sure -- here's the gist of it. Not my prettiest code, it was a lot to squeeze into a fiddle!

SVG Interaction Strawman Fiddle

gloddream commented 4 years ago

Sure -- here's the gist of it. Not my prettiest code, it was a lot to squeeze into a fiddle!

SVG Interaction Strawman Fiddle

when i add a barnote, get the errors: write-SVGInteraction.js:54 Uncaught TypeError: Cannot read property 'style' of null note.attrs.el is null, when the node is barnote...... @gristow

stephaneeybert commented 2 years ago

@gristow It seems like the fiddle is not working any longer. There is nothing in the console log even when mouse hovering and clicking in the SVG context. I'm on Version 99.0.4844.82 (Official Build) (64-bit). The fiddle console shows: "<a class='gotoLine' href='#221:3'>221:3</a> Uncaught ReferenceError: Vex is not defined"

stephaneeybert commented 2 years ago

@0xfe I cannot see any getElem() method on the Vex.Flow.StaveNote type.

I'm on vexflow ^3.0.9 at the moment.

stephaneeybert commented 2 years ago

@gristow Your running version 404s.

stephaneeybert commented 2 years ago

I can display a popup menu right above a note with the following:

@HostListener('click', ['$event'])
onSheetEvent(event: PointerEvent) {
  event.preventDefault();
  event.stopPropagation();
  this.createPopupMenu(event.clientX, event.clientY);
}

But I don't know how to retrieve the logical note index.

Do I need to create a bounding box with top, left, bottom, right coordinates for each note and then traverse all the notes of the soundtrack to find the one matching the click coordinates ?

That would be very slow and unresponsive.

gristow commented 2 years ago

@stephaneeybert the fiddle being down is related to #1354, I've fixed the fiddle so it runs again.

Do I need to create a bounding box with top, left, bottom, right coordinates for each note and then traverse all the notes of the soundtrack to find the one matching the click coordinates ?

Yes, this is the general approach. I memoize the bounding boxes on each render of a score into an array with pointers to the object in question, myself. At the moment I just iterate over the array finding matching coordinates. The lookup time is negligible. If you have such a huge number of elements that lookup time is significant, you could memoize as an array sorted by x or y, and then use a binary search. (Though, I really think the lookup is unlikely to be the performance bottleneck.)

stephaneeybert commented 2 years ago

@gristow Thank you for your advice. I shall go for that design then. Indeed it should not be too costly. I still wonder if that is how an SVG application like https://app.sketchup.com is working too. Or if there is a more direct way of linking an event to an SVG object.

ronyeh commented 2 years ago

FYI: I updated @gristow 's example to work with VexFlow 4.0.1, which we released recently:

https://jsfiddle.net/39gLvqwm/

In VexFlow 4, we can import types as follows:

const { Renderer, Formatter, Stave, StaveNote, Voice } = Vex.Flow;

const renderer = new Renderer(div, Renderer.Backends.SVG);
...

This way, we don't need to use the VF. prefix in every line of code.

The above approach works with script tags in the CommonJS style. If you are using ES modules or TypeScript, you'll use the import keyword and most likely a bundler like webpack or esbuild.

stephaneeybert commented 2 years ago

Am I missing something ? I need to build bounding boxes to later on match a click position against them.

I create first the bounding boxes:

private boundingYs?: Array<BoundingY>;
this.boundingYs = this.sheetService.collectBoundingBoxes(soundtrack);

   public collectBoundingBoxes(soundtrack: Soundtrack): Array<BoundingY> {    
     const boundingYs: Array<BoundingY> = new Array();    
      for (const track of soundtrack.getSortedTracks()) {    
        for (const measure of track.getSortedMeasures()) {    
          if (measure.placedChords) {    
           let bottom: number | undefined;    
           const boundingXs: Array<BoundingX> = new Array();    
           for (const placedChord of measure.getSortedChords()) {    
              if (placedChord.staveNote) {    
                const box: Vex.Flow.BoundingBox = placedChord.staveNote.getBoundingBox();    
               const right = this.svgXToBrowser(soundtrack, box.getX() + box.getW());    
               bottom = this.svgYToBrowser(soundtrack, box.getY() + box.getH());      
               if (right) {    
                 boundingXs.push(new BoundingX(right, placedChord.index));    
               }    
              }    
            }    
           if (bottom) {    
             boundingYs.push(new BoundingY(bottom, measure.index, track.index, boundingXs));    
             console.log(boundingXs);      
             console.log("bottom: " + bottom);    
           }    
          }                                                                                                                                                   
        }                                                                                                                                                     
      }                                                                                                                                                       
     return boundingYs;                                                                                                                                      
   }

   public svgXToBrowser(soundtrack: Soundtrack, svgX: number): number | undefined {
     if (soundtrack.sheetContext != null) {
       const svgMarginLeft: number = soundtrack.sheetContext.svg.getBoundingClientRect().left;
       return svgMarginLeft + svgX;
     }
   }

   public svgYToBrowser(soundtrack: Soundtrack, svgY: number): number | undefined {
     if (soundtrack.sheetContext != null) {
       const svgMarginTop: number = soundtrack.sheetContext.svg.getBoundingClientRect().top;
       return svgMarginTop + svgY;
     }
   }

Then I can search for the correct element:

   public locateMeasureAndChord(boundingYs: Array<BoundingY>, x: number, y: number): [number, number, number] {
     console.log(boundingYs);
     for (const boundingY of boundingYs) {
       console.log('bottom: ' + boundingY.bottom);
       if (y < boundingY.bottom) {
         for (const boundingX of boundingY.boundingXs) {
           console.log('right: ' + boundingX.right);
           if (x < boundingX.right) {
             return [ boundingY.trackId, boundingY.measureId, boundingX.placedChordId ];
           }
         } 
       }
     } 
     return [-1, -1, -1];
    }

And finally I get the click position:

    @HostListener('click', ['$event'])                                             
    onSheetEvent(event: PointerEvent) {                                             
      event.preventDefault();                                             
      event.stopPropagation();                                             
      this.createPopupMenu(event.clientX, event.clientY);                                             
    }
gristow commented 2 years ago

@stephaneeybert -- it looks like there's an error in your svgXToBrowser and svgYToBrowser functions. The browser click event and SVG coordinates are different both in terms of origin and scale. So, you'll need to use a matrix transform.

See this stack overflow or the getCoords function in this fiddle.

stephaneeybert commented 2 years ago

@gristow All right ! Thanks for the feedback and the example !

stephaneeybert commented 2 years ago

@gristow I now notice my logic is faulty as I need to sort my array first before using it.

stephaneeybert commented 2 years ago

It looks like I didn't need to use any svgYToBrowser() method. my application behaves all right without it. I just had to add the possible scroll offset.

The only remaining issue I could have is when the screen is resized. In that case my scrolling offset becomes out of sync.

Maybe doing the coordinates conversion using your suggested matrixTransform() method would solve that too ? I tried this implementation, but the soundtrack.sheetContext.svg.createSVGPoint() method is undefined on the object soundtrack.sheetContext.svg giving me an error at compile time. And the soundtrack.sheetContext.svg.getScreenCTM() method is undefined on the object soundtrack.sheetContext.svg as well.

private screenToSVG(soundtrack: Soundtrack, screenX: number, screenY: number) : SVGPoint | undefined {    
   if (soundtrack.sheetContext != null) {    
     const svgPoint: SVGPoint = soundtrack.sheetContext.svg.createSVGPoint();    
     svgPoint.x = screenX;    
     svgPoint.y = screenY;                                                                                                                                 
     return svgPoint.matrixTransform(soundtrack.sheetContext.svg.getScreenCTM().inverse());                                                                
   }                                                                                                                                                       
 }
gristow commented 2 years ago

Sorry -- I have no idea what soundtrack.sheetContext is, that's not part of the VexFlow code base, so I can't help with its being undefined.

stephaneeybert commented 2 years ago

The soundtrack.sheetContext holds the SVG context:

    soundtrack.sheetContext = this.renderSVGContext(id, sheetWidth, sheetHeight);

    private renderSVGContext(id: string, width: number, height: number): any {    
      const domElement: HTMLElement | null = document.getElementById(id);    
      if (domElement != null) {    
        const renderer: Vex.Flow.Renderer = new Vex.Flow.Renderer(domElement, Vex.Flow.Renderer.Backends.SVG);    
        renderer.resize(width, height);    
        const sheetContext: any = renderer.getContext();    
        return sheetContext;    
      } else {    
        throw new Error('The sheet context could not be created');    
      }    
    }

Sorry for that missing information.

angrykoala commented 2 years ago

Trying the fiddle https://jsfiddle.net/ronyeh/39gLvqwm/ It works fine with normal StaveNote, but fails if I try to run change a note into a TabNote with the following error:

Uncaught TypeError: Cannot read properties of undefined (reading 'style')
    at SVGInteraction.makeInteractive

It seems like note.attrs.el is not defined for TabNotes. Not sure if I'm missing something to be able to have input events on tabs.

Here is the updated fiddle: https://jsfiddle.net/xk0h257o/4/

AaronDavidNewman commented 2 years ago

I think we just need to add this to TabNote draw method? This will also wrap the SVG group around the note with an the ID of the tabnote, which is how Smoosic maps DOM to music.

    // ctx.openGroup('tabnote', undefined, { pointerBBox: true });
    this.setAttribute('el', ctx.openGroup('tabnote', this.getAttribute('id'), { pointerBBox: true }));

image

mscuthbert commented 2 years ago

I'd prefer that Vexflow not alter the current pointer-events system -- there are other users of Vexflow who have added their own UI/UX to it and it seems like scope-creep. What is within the Vexflow remit, I think, is making it so that there are Groups and other selectors so that using software can access these elements and attach pointer-events and eventListeners to them as needed.

AaronDavidNewman commented 2 years ago

@mscuthbert the pointer-events attribute is already on tab note. My change only adds the id attribute to the group, consistent with StaveNote.

I have no issue with removing it, except that it is there currently. I don't use either TabNote or pointer-events in Smoosic.

mscuthbert commented 2 years ago

@mscuthbert the pointer-events attribute is already on tab note. My change only adds the id attribute to the group, consistent with StaveNote.

Ah, my bad -- yes, if it makes things consistent. Is the overall title of "Input events" agreed to be too broad to consider implementing? Should we move setting pointer-events to its own issue?

AaronDavidNewman commented 2 years ago

Is the overall title of "Input events" agreed to be too broad to consider implementing? Should we move setting pointer-events to its own issue?

I think pointer events are fine, and we can close this 6 year old issue.

attrs.element isn't being set on tab notes. I'll create a separate issue for that.