Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: provide option to refresh range axis label #766

Merged
merged 9 commits into from
Oct 8, 2020
Merged

feat: provide option to refresh range axis label #766

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
4f00d2f
feat: provide option to refresh range axis label
hlyang397 Aug 26, 2020
b9abd4c
refactor: move updateRangeAxis to zoomBarsOptions
hlyang397 Aug 27, 2020
9688034
refactor: use configs object instead of boolean as parameter
hlyang397 Aug 28, 2020
056614f
Merge branch 'master' into scalable-y-value
hlyang397 Sep 11, 2020
a224ab3
refactor: move updateWhenZooming option to be under axis option
hlyang397 Sep 11, 2020
180b0b5
Merge branch 'master' into scalable-y-value
hlyang397 Sep 16, 2020
387c68b
Merge branch 'master' into scalable-y-value
hlyang397 Sep 21, 2020
2cd37ad
refactor: change updateWhenZooming option to zoomBar.updateRangeAxis
hlyang397 Oct 6, 2020
858b62f
Merge branch 'master' into scalable-y-value
hlyang397 Oct 6, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion packages/core/src/configuration.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ const axisChart: AxisChartOptions = Tools.merge({}, chart, {
top: {
enabled: false,
type: ZoomBarTypes.GRAPH_VIEW
}
},
updateRangeAxis: false
} as ZoomBarsOptions
} as AxisChartOptions);

Expand Down
4 changes: 4 additions & 0 deletions packages/core/src/interfaces/components.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ export interface ZoomBarsOptions {
* currently only the top position is supported
*/
top?: ZoomBarOptions;
/**
* whether keep updating range axis in real time while zoom domain is changing
*/
updateRangeAxis?: boolean;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've already commented on this before, not sure where my comment went (usually they disappear if you force-push over the branch and the lines that were commented on get removed).

This seems like it should just be flag under:

{
  axes: {
      "top|left|bottom|right": {
         update: boolean;
      }
  }
}

whether it needs to be called update or updateWhenZooming depends on whether this only applies to zooming or not

Copy link
Contributor Author

@hlyang397 hlyang397 Sep 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I will change it to updateWhenZooming since the axis won't change when zoom bar is disabled.
Updated in a224ab3.

}

/**
Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/services/scales-cartesian.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -416,12 +416,18 @@ export class CartesianScales extends Service {
let allDataValues;
// If the scale is stacked
if (axisOptions.stacked) {
const dataValuesGroupedByKeys = this.model.getDataValuesGroupedByKeys();
const dataValuesGroupedByKeys = this.services.zoom.filterDataForRangeAxis(
this.model.getDataValuesGroupedByKeys(),
true
hlyang397 marked this conversation as resolved.
Show resolved Hide resolved
);

allDataValues = dataValuesGroupedByKeys.map((dataValues) =>
sum(values(dataValues) as any)
);
} else {
allDataValues = displayData.map((datum) => datum[mapsTo]);
allDataValues = this.services.zoom
.filterDataForRangeAxis(displayData)
.map((datum) => datum[mapsTo]);
}

if (scaleType !== ScaleTypes.TIME && includeZero) {
Expand Down
64 changes: 64 additions & 0 deletions packages/core/src/services/zoom.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,47 @@
// Internal Imports
import { AxisPositions, ScaleTypes } from "../interfaces";
import { Service } from "./service";
import { Tools } from "../tools";

// D3 imports
import { extent } from "d3-array";

export class Zoom extends Service {
isZoomBarEnabled() {
// CartesianScales service is only available in axis charts
if (!this.services.cartesianScales) {
return false;
}

// @todo - need to update this if zoom bar in other position (bottom, left, right) is supported
// check configuration
if (
!Tools.getProperty(
this.model.getOptions(),
"zoomBar",
"top",
"enabled"
)
) {
return false;
}

// @todo - Zoom Bar only supports main axis at BOTTOM axis and time scale for now
this.services.cartesianScales.findDomainAndRangeAxes(); // need to do this before getMainXAxisPosition()
const mainXAxisPosition = this.services.cartesianScales.getMainXAxisPosition();
const mainXScaleType = Tools.getProperty(
this.model.getOptions(),
"axes",
mainXAxisPosition,
"scaleType"
);

return (
mainXAxisPosition === AxisPositions.BOTTOM &&
mainXScaleType === ScaleTypes.TIME
);
}

// get display data for zoom bar
// basically it's sum of value grouped by time
getZoomBarData() {
Expand Down Expand Up @@ -64,4 +100,32 @@ export class Zoom extends Service {
extent(zoomBarData, (d: any) => d[domainIdentifier])
);
}

// filter out data not inside zoom domain
// to get better range value for axis label
filterDataForRangeAxis(displayData: object[], stacked = false) {
const zoomDomain = this.model.get("zoomDomain");
const isUpdateRangeAxis = Tools.getProperty(
hlyang397 marked this conversation as resolved.
Show resolved Hide resolved
this.model.getOptions(),
"zoomBar",
"updateRangeAxis"
);
if (this.isZoomBarEnabled() && isUpdateRangeAxis && zoomDomain) {
const domainIdentifier = stacked
? "sharedStackKey"
: this.services.cartesianScales.getDomainIdentifier();
const filteredData = displayData.filter(
(datum) =>
new Date(datum[domainIdentifier]) >= zoomDomain[0] &&
new Date(datum[domainIdentifier]) <= zoomDomain[1]
);
// if no data in zoom domain, use all data to get full range value
// so only return filteredData if length > 0
if (filteredData.length > 0) {
return filteredData;
}
}
// return original data by default
return displayData;
}
}