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.16k stars 275 forks source link

point.update doesn't redraw the point #498

Closed evfinkn closed 12 months ago

evfinkn commented 1 year ago

Updating a point's color, labelText, and editable attributes using point.update doesn't redraw the point on the zoomview nor overview. If you seek to a different time and then back to the point, the point will be updated on the zoomview, but the overview remains unchanged. Note that updating the point's time works correctly, as does updating a segment.

chrisn commented 1 year ago

Thanks for reporting this, I'll investigate. Which Peaks,js version are you using?

evfinkn commented 1 year ago

Sorry, I should've specified. I'm experiencing this on the latest commit to master. Not sure what version it stopped working in since I updated to latest from version 2.1.0.

evfinkn commented 1 year ago

Actually, segment's editable and labelText attributes have the same issue of not updating using segment.update. color, startTime, and endTime are updated correctly.

chrisn commented 1 year ago

This is really helpful feedback, thanks.

I'll need to rethink this change. In 2.1.0, updates cause the point/segment to be deleted and re-created, which was simple to implement and meant that updates would always work.

In the 3.0.0-beta development I changed this so that points/segments don't get deleted but instead have an internal update() method that gets called when attributes are changed. The new way is more complex and not fully implemented yet.

chrisn commented 1 year ago

This should be fixed in v3.0.0-beta.12. The marker update() methods are now called correctly. If you're using custom markers, the migration guide explains how to update your code.

One limitation is that changing editable is not currently implemented, but I'm not sure how important that is.

evfinkn commented 1 year ago

Thanks for the fix. I update editable so that the drag markers can be hidden and shown, but it's not a big deal if it's not a priority fix 👍

chrisn commented 1 year ago

Thanks, I'll see what I can do...

chrisn commented 12 months ago

Changing segment editable should now be working in v3.0.0-beta.13.

evfinkn commented 12 months ago

Works great. Thanks!