katspaugh / wavesurfer.js

Audio waveform player
https://wavesurfer.xyz
BSD 3-Clause "New" or "Revised" License
8.63k stars 1.62k forks source link

clearRegions method has bug #3798

Open panhe-xue opened 1 month ago

panhe-xue commented 1 month ago

Bug description

The regions array is not cleared after the clearRegions method is called

Environment

Minimal code snippet

Expected result

Obtained result

Screenshots

image

panhe-xue commented 1 month ago
image
panhe-xue commented 1 month ago

this.regions.forEach((region) => (region && region.remove()))

katspaugh commented 1 month ago

Hmm, weird. Regions are supposed to be removed from the list on the remove event:

https://github.com/katspaugh/wavesurfer.js/blob/main/src/plugins/regions.ts#L581

panhe-xue commented 1 month ago

So we can only remove regions using events, not clearRegions? Because clearRegions is going to be a problem next time.

panhe-xue commented 1 month ago

Hmm, weird. Regions are supposed to be removed from the list on the remove event:

https://github.com/katspaugh/wavesurfer.js/blob/main/src/plugins/regions.ts#L581

So we can only remove regions using events, not clearRegions? Because clearRegions is going to be a problem next time

katspaugh commented 1 month ago

No, the event is emitted when a region is removed. So when clear regions removes each region, it's supposed to emit a remove event and get removed from the list.

panhe-xue commented 1 month ago

No, the event is emitted when a region is removed. So when clear regions removes each region, it's supposed to emit a remove event and get removed from the list.

I see what you're said, How do I know when clearRegions has cleared all of them

panhe-xue commented 1 month ago

No, the event is emitted when a region is removed. So when clear regions removes each region, it's supposed to emit a remove event and get removed from the list.

I see what you're said, How do I know when clearRegions has cleared all of them

I can only use other variables to control, I can't get this moment from an event???

katspaugh commented 1 month ago

You don't need to use the event. All I'm saying is that clearRegions is supposed to remove each region from the this.regions array thanks to an internal remove event subscription. If it doesn't, there might be a bug.

panhe-xue commented 1 month ago

clearRegions

When clearRegions called, you should make sure that addRegions cannot be executed, otherwise it will get messy

katspaugh commented 1 month ago

I see. You can probably call addRegion with a delay (setTimeout) as a workaround.

panhe-xue commented 1 month ago

That doesn't work, I think of a way, could you have a look?

` public clearRegions() { // this.regions.forEach((region) => region.remove()) const regions = cloneDeep(this.regions) regions.forEach((region) => region.remove()) }

` That doesn't work, I think of a way, could you have a look?

` public clearRegions() { // this.regions.forEach((region) => region.remove()) const regions = cloneDeep(this.regions) this.regions = [] regions.forEach((region) => region.remove()) }

`