Region add/remove issues

[eamodio]

If I set the chart.regions directly things seem to work fine, but I'm trying to dynamically add/remove "classes" of regions.

I am using regions as both "markers" and shaded regions and I have multiple different types of regions each with their own "class".

When I try to update a set of regions for a single "class" using chart.regions.remove and then chart.regions.add things don't seem to work properly. Sometimes it seems like the remove call removes markers that don't have that class, but then other times seems to fail to remove even markers of that class as I see them keep overlaying on top of one another.

Here is an example of my code:

chart.regions.remove({ class: 'visible-area' });
if (this.visibleDays == null) return;

chart.regions.add({
	axis: 'x',
	start: this.visibleDays.bottom,
	end: this.visibleDays.top,
	class: 'visible-area',
} satisfies RegionOptions);

Should this work? Or am I not understanding how the remove/add calls are supposed to work?

[netil]

@eamodio, note that .regions.remove() argument type isn't correct in your example.

The key name should be classes, not class and the key value also should be an array.

chart.regions.remove({
    classes: ['visible-area']
});

So, from the chart.regions.remove({ class: 'visible-area' }); call, is considered no valid param is given.
Hence all regions will be removed.

checkout the example:

• https://jsfiddle.net/netil/tm6wea8h/

[eamodio]

@netil Thanks -- that seems to fix a lot of the issues I was seeing.

It looks like the typings are wrong for the regions.remove:
https://github.com/naver/billboard.js/blob/master/types/chart.d.ts#L100-L119

I think the issue I am still seeing is related to the animation when a region comes into view. Is there a way to turn that off?

I am trying to update a region while scrolling another list, and it seems to kind of hide and show the region almost sporadically

Here I am scrolling down a line at a time, and it should be updating the region on each one, but something odd is happening.

[netil]

@eamodio, thanks for inform the incorrect type definition.

It looks like the typings are wrong for the regions.remove:
https://github.com/naver/billboard.js/blob/master/types/chart.d.ts#L100-L119

Will be fixed ASAP.

I think the issue I am still seeing is related to the animation when a region comes into view. Is there a way to turn that off?

The easiest way is turning off the transition by specifying transition.duration=0, but it will affect entire chart updates.
Checkout the below example.

• https://jsfiddle.net/netil/tm6wea8h/2/

Just turning off on grid, it should probably add some new option to grid API to enable/disable the transition.

For the workaround, you can disable transition before the grid API call, and then re-enable it after the grid API call.

• https://jsfiddle.net/netil/tm6wea8h/7/

In this case you can control when to use transition and when to not.

[eamodio]

@netil Thanks! Turning the transition off did the trick. Yeah, it would be nice for finer-grained control on that if possible (e.g. just for the regions).

I also don't know if it would help optimize layout/repaint, but it could be nice to have a regions.update() method, that combines both add and remove

[eamodio]

@netil Thanks for the typing updates. Though I noticed an issue, with the new changes:

The RegionOptions in chart.d.ts doesn't match the new options.

export interface RegionOptions {
	axis?: string;
	start?: string | number | Date;
	end?: string | number | Date;
	class?: string;
}

	regions: {
		/**
		 * Update regions.
		 * @param regions Regions will be replaced with this argument. The format of this argument is the same as regions.
		 */
		(regions: Array<{
			axis: "x" | "y" | "y2",
			start?: number | Date,
			end?: number | Date,
			class?: string
		}>): void;

		/**
		 * Add new region. This API adds new region instead of replacing like regions.
		 * @param grids New region will be added. The format of this argument is the same as regions and it's possible to give an Object if only one region will be added.
		 */
		add<T = {
			axis: "x" | "y" | "y2",
			start?: number | Date,
			end?: number | Date,
			class?: string
		}>(regions: T | T[]): void;

		/**
		 * Remove regions. This API removes regions.
		 * @param args This argument should include classes. If classes is given, the regions that have one of the specified classes will be removed. If args is not given, all of regions will be
		 * removed.
		 */
		remove(args?: { classes: string[] }): void;
	};

[netil]

@eamodio thanks for reporting.

[eamodio]

Related to my comment here: #3037 (comment)

Thoughts on a regions.update() method? I am seeing some performance lag/issues since I am trying to update the regions very frequently.

My use-case is here: gitkraken/vscode-gitlens#2477

[oestrogen]

I should probably open a new issue, but just to get this down in writing:

In both Billboard.js (and C3), there's an undocumented region option, opacity. That's missing from the type definition. I think the option should either be documented and added to the type definition or removed.

The setting of the opacity seems to happen on line 66 in region.ts:

.style("fill-opacity", d => (isValue(d.opacity) ? d.opacity : null))

[netil]

@eamodio

Thoughts on a regions.update() method? I am seeing some performance lag/issues since I am trying to update the regions very frequently.

Currently, when call grid APIs it goes redrawing the chart. I'll be refactoring only to perform on grid related elements.

[eamodio]

Sweet - looking forward to checking it out!

[netil]

@eamodio you can use nightly, which is the latest build from the master branch and the purpose is to give chance to test before the official release.

• https://github.com/naver/billboard.js#nightly-version

[eamodio]

@netil I've tried out the latest version (3.8.0-next.1) and things look pretty good, but I am seeing an issue where when I update a particular region, all the other regions get an extra <rect> element added to them

In the above case, I was updating the visible-area region, by calling regions.remove({ classes: ['visible-area'] }) then regions.add() with the new visible-area regions -- as can be seen here

And because I'm updating those visible regions on scroll, it generates LOTS of those extra <rect> elements which slows things down pretty quickly.

It seems like it might be related to this code:

billboard.js/src/ChartInternal/internals/region.ts

Lines 46 to 48 in c8e1a92

	list
	.append("rect")
	.style("fill-opacity", "0");

[netil]

@eamodio confirmed the issue. Seems having some side-effect from the #3147 issue.

[netil]

released 3.8.0-next.2.

[eamodio]

@netil looks great! TY