search

bbc / peaks.js

JavaScript UI component for interacting with audio waveforms
https://waveform.prototyping.bbc.co.uk
GNU Lesser General Public License v3.0
3.2k stars 279 forks source link

Add logger to options #417

Closed rowild closed 2 years ago

rowild commented 2 years ago

It seems that without logger defined in the options object, an error is thrown:

Uncaught TypeError: this._peaks.logger is not a function
    at WaveformZoomView.setZoom (peaks.js:14546)
    at zoomable-waveform.html:160
    at peaks.js:16876
    at peaks.js:16162
    at Worker.listener (peaks.js:16125)

Adding a logger definition makes the example work again.

Before submitting a pull request, please read our contributor guidelines.

Pull requests that don't follow the guidelines risk not being accepted.

rowild commented 2 years ago

Converted to draft, because I wonder if this is a use case, for which a test would be helpful...

chrisn commented 2 years ago

Sorry if this is a newly introduced bug. I haven't looked into it yet, but a test case to reproduce it would be welcome.

rowild commented 2 years ago

@chrisn No problem! (Not sure if it is new, I think it didn't work before, either...) However, the problem is me and tests. I simply do not know how to write one. I must leave this decision to you – I am very sorry about not being able to help here!

chrisn commented 2 years ago

I have fixed the bug and added some test cases. The issue was that the logger was not being correctly initialised. It shouldn't be necessary to provide a logger config option, so I removed that change.

rowild commented 2 years ago

Thank you very much, @chrisn !