Peaks.init() options structure

[chrisn]

We currently have options that are specific to either the zoomview or overview waveform, e.g., zoomWaveformColor, overviewWaveformColor, overviewHighlightColor, and overviewHighlightOffset.

As we add more options to control waveform rendering, using a single flat structure to specify options to Peaks.init() becomes increasingly awkward.

For example, if we implement a colour change to show played and unplayed regions (#181), we would need to add overviewPlayedWaveformColor and zoomviewPlayedWaveformColor options.

#373 suggests adding a hideAxis option. If we want to set this independently for zoomview and overview waveforms, we would need to add hideOverviewAxis and hideZoomviewAxis options.

We could instead more cleanly add new view-specific options, by adding new zoomview and overview sub-objects to the options, like this:

options = {
  zoomview: {
    waveformColor: ...,
    playedWaveformColor: ...,
    hideAxis: ...
  },
  overview: {
    waveformColor: ...,
    playedWaveformColor: ...,
    hideAxis: ...
  }
}

We could go further and adjust the containers option, so where we currently use:

options = {
  containers: {
    zoomview: ...,
    overview: ...
  }
  // etc
}

we could instead write:

options = {
  zoomview: {
    container: ...,
    // other zoomview-speciifc options, as above
  },
  overview: {
    container: ...,
    // other overview-speciifc options, as above
  },

  // etc
}

Some options currently affect both zoomview and overview waveforms, such as playheadColor and playheadTextColor. This new structure could allow us to easily make these view-specific:

options = {
  zoomview: {
    container: ...,
    playheadColor: ...,
    playheadTextColor: ...,
  },
  overview: {
    container: ...,
    playheadColor: ...,
    playheadTextColor: ...,
  },
}

The top-level option could still be used to set a default that applies to both views, unless overridden by a view-specific option:

options = {
  zoomview: {
    container: ...,
    // zoomview uses default playheadColor
  },
  overview: {
    container: ...,
    playheadColor: ..., // Overrides the default
  },

  playheadColor: ..., // Default value
}

Some options affect the entire Peaks instance, so would only be available as top-level options: mediaElement, dataUri, webAudio, points, segments, etc.

I would keep backwards compatibility with existing option values, to avoid breaking applications.

What do you think? Feedback welcome!

[tscz]

Hi @chrisn, I really like your idea of using a top-level default options and specific overrides for zoomview/overview options. It makes it quite readable and explicit. Maybe a very subjective matter, but seeing your example with the options object, I would write the default values first and then the specific overrides, but just a matter of taste, I guess.

More importantly, this would go into the direction breaking change, even though you are able to keep backward compatibility. To make life easier regarding backwards compatibility and also to offer the new way for clients, is this maybe an option to offer the init of Peaks in a different way? Like a factory function returning a promise? Or an event based config like other APIs are doing it (i.e. https://www.ag-grid.com/javascript-grid/grid-interface/#grid-options)?

[tscz]

Hi @chrisn,

also something came into my mind: At work we once had the issue of quite complex options in a typescript application, whose option parameters were isolated at the beginning but later became inter-related and were functionally bundled ("if you want X to be enabled, then you should also enable Y. Also then Z should only be either this or that."). We used builders for it, which had additional functional verifications and put the business rules into code. It was well received by the api users. It might look like:

Peaks.withDefaultOptions({
  keyboard: true,
  pointMarkerColor: "#006eb0",
  overviewWaveformColor: "#cccccc",
  showPlayheadTime: true
  ...
})
.withZoomview({
    container: ...,
    playheadColor: ...,
    playheadTextColor: ...,
})
.withCustomPlayer({
...
}).init().then(peaks => {...}).catch(error => {...});

Of course, this a great help more for Typescript users than Javascript (IDE support like code completion, typed sub-options, jsdoc comments, ...).

[jdelStrother]

I would keep backwards compatibility with existing option values, to avoid breaking applications.

Are you supporting them forever or should they be marked as deprecated? I'm just updating the typescript defs and wondered about adding something like

diff --git a/peaks.js.d.ts b/peaks.js.d.ts
index 9d1bf3a..bab8c02 100644
--- a/peaks.js.d.ts
+++ b/peaks.js.d.ts
@@ -49,17 +49,22 @@ declare module 'peaks.js' {
   }

   interface SingleContainerOptions {
-    /** Container element for the waveform views */
+    /**
+     * @deprecated use options.<view>.container
+     */
     container: HTMLElement;
   }

   interface ViewContainerOptions {
+    /**
+     * @deprecated use options.<view>.container
+     */
     containers: {
       /** Container element for the overview (non-zoomable) waveform view */
       overview?: HTMLElement | null;
       /** Container element for the zoomable waveform view */
       zoomview?: HTMLElement | null;
     }
   }

[thom4parisot]

I would separate Peaks options and views options, with something like:

import Peaks from 'peaks.js'
import { Overview, Zoomview } from 'peaks.js/views'
import { BeatPulse } from 'myfancyapp/views

const globalOptions = {
  showPlayheadTime: true
}

const p = await Peaks.init({
  keyboard: true,
  ...globalOptions, 
  views: [
    new Overview({ ...globalOptions, playheadColor: 'red' }),
    new Zoomview({ ...globalOptions, playheadColor: 'blue' }),
    new BeatPulse({ colors: ['yellow', 'green', 'purple'] })
  ]
})