Merge pull request #1446 from rvilarl/fix/1445

Refactor & dynamic styling (step 2 Standardize openGroup)

src/annotation.ts

@@ -241,8 +241,7 @@ export class Annotation extends Modifier {
     // still need to save context state just before this, since we will be
     // changing ctx parameters below.
     this.applyStyle();
-    const classString = Object.keys(this.getAttribute('classes')).join(' ');
-    ctx.openGroup(classString, this.getAttribute('id'));
+    ctx.openGroup('annotation', this.getAttribute('id'));
     ctx.setFont(this.textFont);
 
     const text_width = ctx.measureText(this.text).width;

src/beam.ts

@@ -900,9 +900,7 @@ export class Beam extends Element {
       if (stem) {
         const stem_x = note.getStemX();
         stem.setNoteHeadXBounds(stem_x, stem_x);
-        ctx.openGroup('stem', note.getAttribute('id') + '-stem');
         stem.setContext(ctx).draw();
-        ctx.closeGroup();
       }
     }, this);
   }
@@ -930,7 +928,7 @@ export class Beam extends Element {
         if (lastBeamX) {
           const lastBeamY = this.getSlopeY(lastBeamX, firstStemX, beamY, this.slope);
 
-          this.setAttribute('el', ctx.openGroup('beam'));
+          ctx.openGroup('beam', this.getAttribute('id'));
           ctx.beginPath();
           ctx.moveTo(startBeamX, startBeamY);
           ctx.lineTo(startBeamX, startBeamY + beamThickness);

src/chordsymbol.ts

@@ -706,8 +706,7 @@ export class ChordSymbol extends Modifier {
 
     // We're changing context parameters. Save current state.
     ctx.save();
-    const classString = Object.keys(this.getAttribute('classes')).join(' ');
-    ctx.openGroup(classString, this.getAttribute('id'));
+    ctx.openGroup('chordsymbol', this.getAttribute('id'));
 
     const start = note.getModifierStartXY(Modifier.Position.ABOVE, this.index);
     ctx.setFont(this.textFont);

src/element.ts

@@ -7,7 +7,7 @@ import { Font, FontInfo, FontStyle, FontWeight } from './font';
 import { Registry } from './registry';
 import { RenderContext } from './rendercontext';
 import { Category } from './typeguard';
-import { defined } from './util';
+import { defined, prefix } from './util';
 
 /** Element attributes. */
 export interface ElementAttributes {
@@ -260,6 +260,13 @@ export abstract class Element {
     return this.attrs[name];
   }
 
+  /** Return associated SVGElement. */
+  getSVGElement(suffix: string = ''): SVGElement | undefined {
+    const id = prefix(this.attrs.id + suffix);
+    const element = document.getElementById(id);
+    if (element) return element as unknown as SVGElement;
+  }
+
   /** Set an attribute. */
   // eslint-disable-next-line
   setAttribute(name: string, value: any): this {

src/glyphnote.ts

@@ -52,13 +52,11 @@ export class GlyphNote extends Note {
 
   drawModifiers(): void {
     const ctx = this.checkContext();
-    ctx.openGroup('modifiers');
     for (let i = 0; i < this.modifiers.length; i++) {
       const modifier = this.modifiers[i];
       modifier.setContext(ctx);
       modifier.drawWithStyle();
     }
-    ctx.closeGroup();
   }
 
   draw(): void {

src/ornament.ts

@@ -249,8 +249,7 @@ export class Ornament extends Modifier {
     const stemDir = note.getStemDirection();
     const stave = note.checkStave();
 
-    const classString = Object.keys(this.getAttribute('classes')).join(' ');
-    ctx.openGroup(classString, this.getAttribute('id'));
+    ctx.openGroup('ornament', this.getAttribute('id'));
 
     // Get stem extents
     const stemExtents = note.checkStem().getExtents();

src/stavenote.ts

@@ -1100,20 +1100,20 @@ export class StaveNote extends StemmableNote {
   }
 
   // Draw all key modifiers
-  drawModifiers(): void {
+  drawModifiers(noteheadParam: NoteHead): void {
     const ctx = this.checkContext();
-    ctx.openGroup('modifiers');
     for (let i = 0; i < this.modifiers.length; i++) {
       const modifier = this.modifiers[i];
       const index = modifier.checkIndex();
       const notehead = this._noteHeads[index];
-      const noteheadStyle = notehead.getStyle();
-      notehead.applyStyle(ctx, noteheadStyle);
-      modifier.setContext(ctx);
-      modifier.drawWithStyle();
-      notehead.restoreStyle(ctx, noteheadStyle);
+      if (notehead == noteheadParam) {
+        const noteheadStyle = notehead.getStyle();
+        notehead.applyStyle(ctx, noteheadStyle);
+        modifier.setContext(ctx);
+        modifier.drawWithStyle();
+        notehead.restoreStyle(ctx, noteheadStyle);
+      }
     }
-    ctx.closeGroup();
   }
 
   shouldDrawFlag(): boolean {
@@ -1155,20 +1155,19 @@ export class StaveNote extends StemmableNote {
             (this.flag?.getMetrics().y_shift ?? 0) * (1 - this.getStaveNoteScale());
 
       // Draw the Flag
-      ctx.openGroup('flag', undefined, { pointerBBox: true });
       this.applyStyle(ctx, this.getFlagStyle());
       this.flag?.render(ctx, flagX, flagY);
       this.restoreStyle(ctx, this.getFlagStyle());
-      ctx.closeGroup();
     }
   }
 
   // Draw the NoteHeads
   drawNoteHeads(): void {
     const ctx = this.checkContext();
     this._noteHeads.forEach((notehead) => {
-      ctx.openGroup('notehead', undefined, { pointerBBox: true });
+      ctx.openGroup('notehead', notehead.getAttribute('id'), { pointerBBox: true });
       notehead.setContext(ctx).draw();
+      this.drawModifiers(notehead);
       ctx.closeGroup();
     });
   }
@@ -1190,9 +1189,7 @@ export class StaveNote extends StemmableNote {
     }
 
     if (this.stem) {
-      ctx.openGroup('stem', undefined, { pointerBBox: true });
       this.stem.setContext(ctx).draw();
-      ctx.closeGroup();
     }
   }
 
@@ -1267,15 +1264,12 @@ export class StaveNote extends StemmableNote {
 
     // Apply the overall style -- may be contradicted by local settings:
     this.applyStyle();
-    this.setAttribute('el', ctx.openGroup('stavenote', this.getAttribute('id')));
+    ctx.openGroup('stavenote', this.getAttribute('id'));
     this.drawLedgerLines();
-    ctx.openGroup('note', undefined, { pointerBBox: true });
     if (shouldRenderStem) this.drawStem();
     this.drawNoteHeads();
     this.drawFlag();
     ctx.closeGroup();
-    this.drawModifiers();
-    ctx.closeGroup();
     this.restoreStyle();
     this.setRendered();
   }

src/stavetie.ts

@@ -152,13 +152,7 @@ export class StaveTie extends Element {
       const top_cp_y = (first_y_px + last_y_px) / 2 + cp1 * params.direction;
       const bottom_cp_y = (first_y_px + last_y_px) / 2 + cp2 * params.direction;
 
-      // id probably unnecessary if we save the group to 'el' via setAttribute
-      // let id: string = "";
-      // if (this.notes.first_note) {
-      //   id = this.notes.first_note.getAttribute('id') + '-tie';
-      // }
-      // this.setAttribute('el', ctx.openGroup('stavetie', id));
-      this.setAttribute('el', ctx.openGroup('stavetie'));
+      ctx.openGroup('stavetie', this.getAttribute('id'));
       ctx.beginPath();
       ctx.moveTo(params.first_x_px + first_x_shift, first_y_px);
       ctx.quadraticCurveTo(cp_x, top_cp_y, params.last_x_px + last_x_shift, last_y_px);

src/stem.ts

@@ -210,13 +210,15 @@ export class Stem extends Element {
 
     // Draw the stem
     ctx.save();
+    ctx.openGroup('stem', this.getAttribute('id'), { pointerBBox: true });
     this.applyStyle(ctx);
     ctx.beginPath();
     ctx.setLineWidth(Stem.WIDTH);
     ctx.moveTo(stem_x, stem_y - stemletYOffset + y_base_offset);
     ctx.lineTo(stem_x, stem_y - stemHeight - this.renderHeightAdjustment * stem_direction);
     ctx.stroke();
     this.restoreStyle(ctx);
+    ctx.closeGroup();
     ctx.restore();
   }
 }

src/tabnote.ts

@@ -455,16 +455,14 @@ export class TabNote extends StemmableNote {
     this.setRendered();
     const render_stem = this.beam == undefined && this.render_options.draw_stem;
 
-    this.setAttribute('el', ctx.openGroup('tabnote', this.getAttribute('id'), { pointerBBox: true }));
+    ctx.openGroup('tabnote', this.getAttribute('id'), { pointerBBox: true });
     this.drawPositions();
     this.drawStemThrough();
 
     if (this.stem && render_stem) {
       const stem_x = this.getStemX();
       this.stem.setNoteHeadXBounds(stem_x, stem_x);
-      ctx.openGroup('stem', undefined, { pointerBBox: true });
       this.stem.setContext(ctx).draw();
-      ctx.closeGroup();
     }
 
     this.drawFlag();

tests/stavenote_tests.ts

@@ -403,9 +403,11 @@ function drawBasic(options: TestOptions, contextBuilder: ContextBuilder): void {
 
     // If this is an interactivity test (ui: true), then attach mouseover & mouseout handlers to the notes.
     if (options.params.ui) {
-      const item = note.getAttribute('el') as SVGElement;
-      item.addEventListener('mouseover', colorDescendants(item, 'green'), false);
-      item.addEventListener('mouseout', colorDescendants(item, 'black'), false);
+      const item = note.getSVGElement();
+      if (item) {
+        item.addEventListener('mouseover', colorDescendants(item, 'green'), false);
+        item.addEventListener('mouseout', colorDescendants(item, 'black'), false);
+      }
     }
     ok(note.getX() > 0, 'Note ' + i + ' has X value');
     ok(note.getYs().length > 0, 'Note ' + i + ' has Y values');