Skip to content

Commit

Permalink
feat(xy): mutilayer time axis step 1 (#1326)
Browse files Browse the repository at this point in the history
- adds a hierarchical time axis story
- fix/feat: the small update to actually left-align the horizontal axis ticks
- fix: turned all `[].sort()` calls safe (some API inputs used to be modified in the past; also, brittle to future code changes)
- fix(test): makes certain array comparison tests use `toIncludeSameMembers` instead of `toEqual` so the test remain neutral about the order of array elements (not all arrays need to have a specific order)
- fix: `axisTickLabel` (populated by `Axis.labelFormat`) is now precomputed and used for bounding box calculation and wherever it's needed for the actual axis tick rendering, as the preexisting `label` (populated by `Axis.tickFormat`) was generic, used eg. for tooltip formatting
- test: custom font that's fairly usable in live storybook mode (font for a11y: Atkinson Hyperlegible)
- refactor: swathes of axis related code eg. 350 lines removed from `axis_utils.ts` alone, not counting the deletion of the axis dedupe fun (refactoring is guided by certain objectives such as clarity of data flow, performance, simple types, avoidance of special cases and bails, expressions instead of control structures, succinctness)

BREAKING CHANGE:
- feat: removes the axis deduplication feature
- fix: `showDuplicatedTicks` causes a duplication check on the actual axis tick label (possibly yielded by `Axis.tickLabel` rather than the more general `tickFormat`)
  • Loading branch information
monfera authored Sep 8, 2021
1 parent 16b5546 commit 867b1f5
Show file tree
Hide file tree
Showing 40 changed files with 796 additions and 1,507 deletions.
10 changes: 10 additions & 0 deletions integration/server/webpack.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,16 @@ module.exports = {
},
module: {
rules: [
{
test: /\.(ttf|eot|woff|woff2|svg)$/,
use: {
loader: 'file-loader',
options: {
name: '[name].[ext]',
outputPath: 'fonts/',
},
},
},
{
test: /\.tsx?$/,
loader: 'ts-loader',
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file not shown.
3 changes: 1 addition & 2 deletions packages/charts/api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -642,7 +642,7 @@ export const DEFAULT_TOOLTIP_SNAP = true;
export const DEFAULT_TOOLTIP_TYPE: "vertical";

// @public (undocumented)
export type DefaultSettingsProps = 'id' | 'chartType' | 'specType' | 'rendering' | 'rotation' | 'resizeDebounce' | 'pointerUpdateDebounce' | 'pointerUpdateTrigger' | 'animateData' | 'debug' | 'tooltip' | 'theme' | 'hideDuplicateAxes' | 'brushAxis' | 'minBrushDelta' | 'externalPointerEvents' | 'showLegend' | 'showLegendExtra' | 'legendPosition' | 'legendMaxDepth' | 'ariaUseDefaultSummary' | 'ariaLabelHeadingLevel' | 'ariaTableCaption';
export type DefaultSettingsProps = 'id' | 'chartType' | 'specType' | 'rendering' | 'rotation' | 'resizeDebounce' | 'pointerUpdateDebounce' | 'pointerUpdateTrigger' | 'animateData' | 'debug' | 'tooltip' | 'theme' | 'brushAxis' | 'minBrushDelta' | 'externalPointerEvents' | 'showLegend' | 'showLegendExtra' | 'legendPosition' | 'legendMaxDepth' | 'ariaUseDefaultSummary' | 'ariaLabelHeadingLevel' | 'ariaTableCaption';

// @public (undocumented)
export const DEPTH_KEY = "depth";
Expand Down Expand Up @@ -1872,7 +1872,6 @@ export interface SettingsSpec extends Spec, LegendSpec {
debugState?: boolean;
// @alpha
externalPointerEvents: ExternalPointerEventsSettings;
hideDuplicateAxes: boolean;
minBrushDelta?: number;
noResults?: ComponentType | ReactChild;
onAnnotationClick?: AnnotationClickListener;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -405,8 +405,7 @@ export function geoms(
const angleScale = (x: number) => angleStart + (angleRange * (x - domain[0])) / domainExtent;
const clockwise = angleStart > angleEnd; // todo refine this crude approach

const geomObjects = abstractGeoms
.slice()
return [...abstractGeoms]
.sort((a, b) => a.order - b.order)
.map(({ landmarks, aes }) => {
const at = get(landmarks, 'at', '');
Expand Down Expand Up @@ -465,5 +464,4 @@ export function geoms(
return new Section(x0, y0, x1, y1, lineWidth, strokeStyle);
}
});
return geomObjects;
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,6 @@ export function shapeViewModel(spec: GoalSpec, theme: Theme, chartDimensions: Di
y: margin.top + innerHeight / 2,
};

const pickQuads: PickFunction = (x, y) =>
-innerWidth / 2 <= x && x <= innerWidth / 2 && -innerHeight / 2 <= y && y <= innerHeight / 2
? [bulletViewModel]
: [];

const {
subtype,
base,
Expand Down Expand Up @@ -91,6 +86,11 @@ export function shapeViewModel(spec: GoalSpec, theme: Theme, chartDimensions: Di
angleEnd,
};

const pickQuads: PickFunction = (x, y) =>
-innerWidth / 2 <= x && x <= innerWidth / 2 && -innerHeight / 2 <= y && y <= innerHeight / 2
? [bulletViewModel]
: [];

return {
config: theme.goal,
chartCenter,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,21 +69,14 @@ function getTreesForSpec(
group.push(next);
return map;
}, new Map<string, HierarchyOfArrays>());
return Array.from(groups)
.sort(getPredicateFn(sort))
.map(([groupKey, subData], innerIndex) => ({
name: format(groupKey),
smAccessorValue: groupKey,
style: smStyle,
tree: partitionTree(
subData,
valueAccessor,
layers,
configMetadata.partitionLayout.dflt,
config.partitionLayout,
[{ index: innerIndex, value: String(groupKey) }],
),
}));
return [...groups].sort(getPredicateFn(sort)).map(([groupKey, subData], innerIndex) => ({
name: format(groupKey),
smAccessorValue: groupKey,
style: smStyle,
tree: partitionTree(subData, valueAccessor, layers, configMetadata.partitionLayout.dflt, config.partitionLayout, [
{ index: innerIndex, value: String(groupKey) },
]),
}));
} else {
return [
{
Expand Down
10 changes: 4 additions & 6 deletions packages/charts/src/chart_types/xy_chart/annotations/tooltip.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,11 +24,10 @@ export function computeRectAnnotationTooltipState(
chartRotation: Rotation,
chartDimensions: Dimensions,
): AnnotationTooltipState | null {
// allow picking up the last spec added as the top most or use it's zIndex value
// allow picking up the last spec added as the top most or use its zIndex value
const sortedAnnotationSpecs = annotationSpecs
.filter(isRectAnnotation)
.reverse()
.sort(({ zIndex: a = Number.MIN_SAFE_INTEGER }, { zIndex: b = Number.MIN_SAFE_INTEGER }) => b - a);
.sort(({ zIndex: a = Number.MIN_SAFE_INTEGER }, { zIndex: b = Number.MIN_SAFE_INTEGER }) => a - b);

for (let i = 0; i < sortedAnnotationSpecs.length; i++) {
const spec = sortedAnnotationSpecs[i];
Expand Down Expand Up @@ -69,11 +68,10 @@ export function computeMultipleRectAnnotationTooltipState(
chartRotation: Rotation,
chartDimensions: Dimensions,
): AnnotationTooltipState[] {
// allow picking up the last spec added as the top most or use it's zIndex value
// allow picking up the last spec added as the top most or use its zIndex value
const sortedAnnotationSpecs = annotationSpecs
.filter(isRectAnnotation)
.reverse()
.sort(({ zIndex: a = Number.MIN_SAFE_INTEGER }, { zIndex: b = Number.MIN_SAFE_INTEGER }) => b - a);
.sort(({ zIndex: a = Number.MIN_SAFE_INTEGER }, { zIndex: b = Number.MIN_SAFE_INTEGER }) => a - b);
return sortedAnnotationSpecs.reduce<AnnotationTooltipState[]>((acc, spec) => {
const annotationDimension = annotationDimensions.get(spec.id);
if (!spec.hideTooltips && annotationDimension) {
Expand Down
5 changes: 3 additions & 2 deletions packages/charts/src/chart_types/xy_chart/axes/axes_sizes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@ import { Position } from '../../../utils/common';
import { innerPad, outerPad, PerSideDistance } from '../../../utils/dimensions';
import { AxisId } from '../../../utils/ids';
import { AxisStyle, Theme } from '../../../utils/themes/theme';
import { AxesTicksDimensions } from '../state/selectors/compute_axis_ticks_dimensions';
import { getSpecsById } from '../state/utils/spec';
import { isVerticalAxis } from '../utils/axis_type_utils';
import { AxisViewModel, getTitleDimension, shouldShowTicks } from '../utils/axis_utils';
import { getTitleDimension, shouldShowTicks } from '../utils/axis_utils';
import { AxisSpec } from '../utils/specs';

/** @internal */
export function computeAxesSizes(
{ axes: sharedAxesStyles, chartMargins }: Theme,
axisDimensions: Map<AxisId, AxisViewModel>,
axisDimensions: AxesTicksDimensions,
axesStyles: Map<AxisId, AxisStyle | null>,
axisSpecs: AxisSpec[],
smSpec?: SmallMultiplesSpec,
Expand Down
4 changes: 2 additions & 2 deletions packages/charts/src/chart_types/xy_chart/domains/x_domain.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export function mergeXDomain(
seriesXComputedDomains = computeOrdinalDataDomain(values, identity, false, true);
if (customDomain) {
if (Array.isArray(customDomain)) {
seriesXComputedDomains = customDomain;
seriesXComputedDomains = [...customDomain];
} else {
if (fallbackScale === ScaleType.Ordinal) {
Logger.warn(`xDomain ignored for fallback ordinal scale. Options to resolve:
Expand Down Expand Up @@ -140,7 +140,7 @@ export function findMinInterval(xValues: number[]): number {
if (valuesLength === 1) {
return 1;
}
const sortedValues = xValues.slice().sort(compareByValueAsc);
const sortedValues = [...xValues].sort(compareByValueAsc);
let i;
let minInterval = Math.abs(sortedValues[1] - sortedValues[0]);
for (i = 1; i < valuesLength - 1; i++) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import { Dimensions, Size } from '../../../../../utils/dimensions';
import { Point } from '../../../../../utils/point';
import { AxisStyle } from '../../../../../utils/themes/theme';
import { PerPanelAxisGeoms } from '../../../state/selectors/compute_per_panel_axes_geoms';
import { AxisTick, AxisViewModel, shouldShowTicks } from '../../../utils/axis_utils';
import { AxisTick, TickLabelBounds, shouldShowTicks } from '../../../utils/axis_utils';
import { AxisSpec } from '../../../utils/specs';
import { renderAxisLine } from './line';
import { renderTick } from './tick';
Expand All @@ -25,7 +25,7 @@ export interface AxisProps {
axisSpec: AxisSpec;
size: Size;
anchorPoint: Point;
dimension: AxisViewModel;
dimension: TickLabelBounds;
ticks: AxisTick[];
debug: boolean;
renderingArea: Dimensions;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function renderTickLabel(
ctx: CanvasRenderingContext2D,
tick: AxisTick,
showTicks: boolean,
{ axisSpec: { position, labelFormat }, dimension, size, debug, axisStyle }: AxisProps,
{ axisSpec: { position }, dimension, size, debug, axisStyle }: AxisProps,
) {
const labelStyle = axisStyle.tickLabel;
const tickLabelProps = getTickLabelProps(
Expand Down Expand Up @@ -46,7 +46,7 @@ export function renderTickLabel(
renderText(
ctx,
center,
labelFormat ? labelFormat(tick.value) : tick.label,
tick.axisTickLabel,
{
fontFamily: labelStyle.fontFamily,
fontStyle: labelStyle.fontStyle ?? 'normal',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
// eslint-disable-next-line eslint-comments/disable-enable-pair
/* eslint-disable jest/no-conditional-expect */

import 'jest-extended';
import React from 'react';
import { Store } from 'redux';

Expand Down Expand Up @@ -79,7 +80,6 @@ const settingSpec = MockGlobalSpec.settings({
tooltip: {
type: TooltipType.VerticalCursor,
},
hideDuplicateAxes: false,
theme: {
chartPaddings: { top: 0, left: 0, bottom: 0, right: 0 },
chartMargins: { top: 10, left: 10, bottom: 0, right: 0 },
Expand Down Expand Up @@ -1289,7 +1289,7 @@ describe('Clickable annotations', () => {

expect(onAnnotationClick).toBeCalled();
const callArgs = onAnnotationClick.mock.calls[0][0];
expect(callArgs.rects).toEqual([
expect(callArgs.rects).toIncludeSameMembers([
{
id: 'rect2',
datum: {
Expand Down
165 changes: 0 additions & 165 deletions packages/charts/src/chart_types/xy_chart/state/chart_state.test.ts

This file was deleted.

Loading

0 comments on commit 867b1f5

Please sign in to comment.