review comments

0xfe · May 7, 2021 · abcd8aa · abcd8aa
1 parent 73e9421
commit abcd8aa
Show file tree

Hide file tree

Showing 5 changed files with 37 additions and 90 deletions.
diff --git a/src/stave.ts b/src/stave.ts
@@ -343,14 +343,12 @@ export class Stave extends Element {
     return (y - this.y) / spacing - headroom;
   }
 
-  getYForTopText(line?: number): number {
-    const l = line || 0;
-    return this.getYForLine(-l - this.options.top_text_position);
+  getYForTopText(line: number = 0): number {
+    return this.getYForLine(-line - this.options.top_text_position);
   }
 
-  getYForBottomText(line?: number): number {
-    const l = line || 0;
-    return this.getYForLine(this.options.bottom_text_position + l);
+  getYForBottomText(line: number = 0): number {
+    return this.getYForLine(this.options.bottom_text_position + line);
   }
 
   getYForNote(line: number): number {

diff --git a/src/stavetie.ts b/src/stavetie.ts
@@ -6,10 +6,17 @@
 
 import { Vex } from './vex';
 import { Element } from './element';
-import { FontInfo } from './types/common';
+import { FontInfo, RenderTieParams } from './types/common';
 import { Note } from './note';
 import { Stave } from './stave';
 
+export interface Notes {
+  first_note: Note;
+  last_note: Note;
+  first_indices: number[];
+  last_indices: number[];
+}
+
 export interface StaveTieRenderOptions {
   cp2: number;
   last_x_shift: number;
@@ -21,37 +28,16 @@ export interface StaveTieRenderOptions {
   font: FontInfo;
 }
 
-export interface StaveTieRenderTieParams {
-  direction: number;
-  first_x_px: number;
-  last_x_px: number;
-  last_ys: number[];
-  first_ys: number[];
-}
-
 export class StaveTie extends Element {
   render_options: StaveTieRenderOptions;
 
   protected text?: string;
 
   protected font: FontInfo;
-  protected notes: {
-    first_note: Note;
-    last_note: Note;
-    first_indices: number[];
-    last_indices: number[];
-  };
+  protected notes: Notes;
   protected direction?: number;
 
-  constructor(
-    notes: {
-      first_note: Note;
-      last_note: Note;
-      first_indices: number[];
-      last_indices: number[];
-    },
-    text?: string
-  ) {
+  constructor(notes: Notes, text?: string) {
     /**
      * Notes is a struct that has:
      *
@@ -97,7 +83,7 @@ export class StaveTie extends Element {
    *
    * @param {!Object} notes The notes to tie up.
    */
-  setNotes(notes: { first_note: Note; last_note: Note; first_indices: number[]; last_indices: number[] }): this {
+  setNotes(notes: Notes): this {
     if (!notes.first_note && !notes.last_note) {
       throw new Vex.RuntimeError('BadArguments', 'Tie needs to have either first_note or last_note set.');
     }
@@ -121,7 +107,7 @@ export class StaveTie extends Element {
     return !this.notes.first_note || !this.notes.last_note;
   }
 
-  renderTie(params: StaveTieRenderTieParams): void {
+  renderTie(params: RenderTieParams): void {
     if (params.first_ys.length === 0 || params.last_ys.length === 0) {
       throw new Vex.RERR('BadArguments', 'No Y-values to render');
     }
@@ -165,20 +151,11 @@ export class StaveTie extends Element {
     const ctx = this.checkContext();
     let center_x = (first_x_px + last_x_px) / 2;
     center_x -= ctx.measureText(this.text).width / 2;
-    let stave: Stave;
-    const firstNoteStave = this.notes.first_note ? this.notes.first_note.getStave() : undefined;
-    const lastNoteStave = this.notes.last_note ? this.notes.last_note.getStave() : undefined;
-    if (firstNoteStave) {
-      stave = firstNoteStave;
-    } else if (lastNoteStave) {
-      stave = lastNoteStave;
-    } else {
-      throw new Vex.RERR('NoStave', 'Stave required');
-    }
+    const stave = this.notes.first_note?.getStave() ?? this.notes.last_note?.getStave();
 
     ctx.save();
     ctx.setFont(this.font.family, this.font.size, this.font.weight);
-    ctx.fillText(this.text, center_x + this.render_options.text_shift_x, stave.getYForTopText() - 1);
+    ctx.fillText(this.text, center_x + this.render_options.text_shift_x, stave!.getYForTopText() - 1);
     ctx.restore();
   }
 

diff --git a/src/tabslide.ts b/src/tabslide.ts
@@ -6,9 +6,9 @@
 
 import { Vex } from './vex';
 import { TabTie } from './tabtie';
-import { Note } from './note';
 import { TabNote } from './tabnote';
-import { StaveTieRenderTieParams } from './stavetie';
+import { Notes } from './stavetie';
+import { RenderTieParams } from './types/common';
 
 export class TabSlide extends TabTie {
   static get SLIDE_UP(): number {
@@ -19,33 +19,15 @@ export class TabSlide extends TabTie {
     return -1;
   }
 
-  static createSlideUp(notes: {
-    first_note: Note;
-    last_note: Note;
-    first_indices: number[];
-    last_indices: number[];
-  }): TabSlide {
+  static createSlideUp(notes: Notes): TabSlide {
     return new TabSlide(notes, TabSlide.SLIDE_UP);
   }
 
-  static createSlideDown(notes: {
-    first_note: Note;
-    last_note: Note;
-    first_indices: number[];
-    last_indices: number[];
-  }): TabSlide {
+  static createSlideDown(notes: Notes): TabSlide {
     return new TabSlide(notes, TabSlide.SLIDE_DOWN);
   }
 
-  constructor(
-    notes: {
-      first_note: Note;
-      last_note: Note;
-      first_indices: number[];
-      last_indices: number[];
-    },
-    direction?: number
-  ) {
+  constructor(notes: Notes, direction?: number) {
     /**
      * Notes is a struct that has:
      *
@@ -76,7 +58,7 @@ export class TabSlide extends TabTie {
     this.setNotes(notes);
   }
 
-  renderTie(params: StaveTieRenderTieParams): void {
+  renderTie(params: RenderTieParams): void {
     if (params.first_ys.length === 0 || params.last_ys.length === 0) {
       throw new Vex.RERR('BadArguments', 'No Y-values to render');
     }

diff --git a/src/tabtie.ts b/src/tabtie.ts
@@ -4,37 +4,18 @@
 // This class implements varies types of ties between contiguous notes. The
 // ties include: regular ties, hammer ons, pull offs, and slides.
 
-import { Note } from './note';
-import { StaveTie } from './stavetie';
+import { Notes, StaveTie } from './stavetie';
 
 export class TabTie extends StaveTie {
-  static createHammeron(notes: {
-    first_note: Note;
-    last_note: Note;
-    first_indices: number[];
-    last_indices: number[];
-  }): TabTie {
+  static createHammeron(notes: Notes): TabTie {
     return new TabTie(notes, 'H');
   }
 
-  static createPulloff(notes: {
-    first_note: Note;
-    last_note: Note;
-    first_indices: number[];
-    last_indices: number[];
-  }): TabTie {
+  static createPulloff(notes: Notes): TabTie {
     return new TabTie(notes, 'P');
   }
 
-  constructor(
-    notes: {
-      first_note: Note;
-      last_note: Note;
-      first_indices: number[];
-      last_indices: number[];
-    },
-    text?: string
-  ) {
+  constructor(notes: Notes, text?: string) {
     /**
      * Notes is a struct that has:
      *

diff --git a/src/types/common.d.ts b/src/types/common.d.ts
@@ -95,3 +95,12 @@ export interface RenderContext {
    */
   measureText(text: string): { width: number };
 }
+
+export interface RenderTieParams {
+  direction: number;
+  first_x_px: number;
+  last_x_px: number;
+  last_ys: number[];
+  first_ys: number[];
+}
+