Skip to content

Fix: Forward notify flag in BaseChart.remove() overrides (WGLChart and other subclasses) #484

Description

@coderabbitai

Summary

When BaseChart.remove(notify?: boolean) was updated in PR #482 to call destroyColorLegendFilter(this, notify), some subclass overrides do not forward the notify parameter when calling super.remove(). This means the notify flag is silently dropped, which can cause unintended callbacks in paths that are supposed to be silent (e.g. onColumnRemoved() uses remove(false)).

Problem

In src/charts/WGLChart.js:

remove(notify = true) {
    this.dim.destroy(notify);
    this.app.remove();
    super.remove(); // ← notify not forwarded
}

destroyColorLegendFilter is always invoked with notify = undefined (truthy behaviour depends on implementation) instead of the caller's explicit value.

Fix

All overrides of BaseChart.remove() should accept and forward the notify parameter:

remove(notify = true) {
    this.dim.destroy(notify);
    this.app.remove();
    super.remove(notify); // ← forward notify
}

Any other subclass that overrides remove() should be audited for the same pattern.

References

Metadata

Metadata

Assignees

Labels

No labels
No labels

Type

No type

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions