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.

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(bullet): the tooltip shows up around the drawn part of the chart only #1278

Merged
merged 21 commits into from
Aug 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
21 commits
Select commit Hold shift + click to select a range
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
2 changes: 1 addition & 1 deletion .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ module.exports = {
'unicorn/no-nested-ternary': 0,
'@typescript-eslint/lines-between-class-members': ['error', 'always', { exceptAfterSingleLine: true }],
'no-extra-parens': 'off', // it was already off by default; this line addition is just for documentation purposes
'@typescript-eslint/restrict-template-expressions': 0, // it's OK to use numbers etc. in string templates

/**
*****************************************
Expand All @@ -65,7 +66,6 @@ module.exports = {
'@typescript-eslint/no-unsafe-member-access': 0,
'@typescript-eslint/no-unsafe-return': 0,
'@typescript-eslint/explicit-module-boundary-types': 0,
'@typescript-eslint/restrict-template-expressions': 1,
'@typescript-eslint/restrict-plus-operands': 0, // rule is broken
'@typescript-eslint/no-unsafe-call': 1,
'@typescript-eslint/unbound-method': 1,
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@
* Side Public License, v 1.
*/

import { GOLDEN_RATIO } from '../../../../common/constants';
import { PointObject } from '../../../../common/geometry';
import { GOLDEN_RATIO, TAU } from '../../../../common/constants';
import { PointObject, Radian, Rectangle } from '../../../../common/geometry';
import { cssFontShorthand, Font } from '../../../../common/text_utils';
import { GoalSubtype } from '../../specs/constants';
import { Config } from '../types/config_types';
Expand All @@ -22,9 +22,12 @@ const marginRatio = 0.05; // same ratio on each side
const maxTickFontSize = 24;
const maxLabelFontSize = 32;
const maxCentralFontSize = 38;
const arcBoxSamplePitch: Radian = (5 / 360) * TAU; // 5-degree pitch ie. a circle is 72 steps
const capturePad = 16; // mouse hover is detected in the padding too; eg. for Fitts law

/** @internal */
export interface Mark {
boundingBoxes: (ctx: CanvasRenderingContext2D) => Rectangle[];
render: (ctx: CanvasRenderingContext2D) => void;
}

Expand All @@ -46,6 +49,22 @@ export class Section implements Mark {
this.strokeStyle = strokeStyle;
}

boundingBoxes() {
// modifying with half the line width is a simple yet imprecise method for ensuring that the
// entire ink is in the bounding box; depending on orientation and line ending, the bounding
// box may overstate the data ink bounding box, which is preferable to understating it
return this.lineWidth === 0
? []
: [
{
x0: Math.min(this.x, this.xTo) - this.lineWidth / 2 - capturePad,
y0: Math.min(this.y, this.yTo) - this.lineWidth / 2 - capturePad,
x1: Math.max(this.x, this.xTo) + this.lineWidth / 2 + capturePad,
y1: Math.max(this.y, this.yTo) + this.lineWidth / 2 + capturePad,
},
];
}

render(ctx: CanvasRenderingContext2D) {
ctx.beginPath();
ctx.lineWidth = this.lineWidth;
Expand All @@ -56,13 +75,16 @@ export class Section implements Mark {
}
}

/** @internal */
export const initialBoundingBox = (): Rectangle => ({ x0: Infinity, y0: Infinity, x1: -Infinity, y1: -Infinity });

/** @internal */
export class Arc implements Mark {
protected readonly x: number;
protected readonly y: number;
protected readonly radius: number;
protected readonly startAngle: number;
protected readonly endAngle: number;
protected readonly startAngle: Radian;
Copy link
Collaborator

Choose a reason for hiding this comment

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

👍🏼

protected readonly endAngle: Radian;
protected readonly anticlockwise: boolean;
protected readonly lineWidth: number;
protected readonly strokeStyle: string;
Expand All @@ -87,6 +109,49 @@ export class Arc implements Mark {
this.strokeStyle = strokeStyle;
}

boundingBoxes() {
if (this.lineWidth === 0) return [];

const box = initialBoundingBox();

// instead of an analytical solution, we approximate with a GC-free grid sampler

// full circle rotations such that `startAngle' and `endAngle` are positive
const rotationCount = Math.ceil(Math.max(0, -this.startAngle, -this.endAngle) / TAU);
const startAngle = this.startAngle + rotationCount * TAU;
const endAngle = this.endAngle + rotationCount * TAU;

// snapping to the closest `arcBoxSamplePitch` increment
const angleFrom: Radian = Math.round(startAngle / arcBoxSamplePitch) * arcBoxSamplePitch;
const angleTo: Radian = Math.round(endAngle / arcBoxSamplePitch) * arcBoxSamplePitch;
const signedIncrement = arcBoxSamplePitch * Math.sign(angleTo - angleFrom);

for (let angle: Radian = angleFrom; angle <= angleTo; angle += signedIncrement) {
// unit vector for the angle direction
const vx = Math.cos(angle);
const vy = Math.sin(angle);
const innerRadius = this.radius - this.lineWidth / 2;
const outerRadius = this.radius + this.lineWidth / 2;

// inner point of the sector
const innerX = this.x + vx * innerRadius;
const innerY = this.y + vy * innerRadius;

// outer point of the sector
const outerX = this.x + vx * outerRadius;
const outerY = this.y + vy * outerRadius;

box.x0 = Math.min(box.x0, innerX - capturePad, outerX - capturePad);
box.y0 = Math.min(box.y0, innerY - capturePad, outerY - capturePad);
box.x1 = Math.max(box.x1, innerX + capturePad, outerX + capturePad);
box.y1 = Math.max(box.y1, innerY + capturePad, outerY + capturePad);

if (signedIncrement === 0) break; // happens if fromAngle === toAngle
}

return Number.isFinite(box.x0) ? [box] : [];
}

render(ctx: CanvasRenderingContext2D) {
ctx.beginPath();
ctx.lineWidth = this.lineWidth;
Expand Down Expand Up @@ -124,11 +189,30 @@ export class Text implements Mark {
this.fontSize = fontSize;
}

render(ctx: CanvasRenderingContext2D) {
ctx.beginPath();
setCanvasTextState(ctx: CanvasRenderingContext2D) {
ctx.textAlign = this.textAlign;
ctx.textBaseline = this.textBaseline;
ctx.font = cssFontShorthand(this.fontShape, this.fontSize);
}

boundingBoxes(ctx: CanvasRenderingContext2D) {
if (this.text.length === 0) return [];

this.setCanvasTextState(ctx);
const box = ctx.measureText(this.text);
Comment on lines +201 to +202
Copy link
Member

Choose a reason for hiding this comment

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

Can we reuse measureText that also brings in the font, size etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought about it, but these bounding box methods are mirrors of the actual rendering code (this.setCanvasTextState(ctx) and precise input is fully shared), so it's already as close to font, size etc. as possible. The function measureText in src/common assumes text Box objects which are only used in most partition charts, due to the multirow layout, where its purpose is, adaptive font sizing and line breaks, so it's not a close fit currently.

It'd be a good idea to eventually migrate bullet graphs to that though: the user would specify the bounding box into which the bullet/goal titles must fit. It'd be a larger, API-involving task. Also, then the bounding box would be given, because that's what the user wants to fill.

If you meant CanvasTextBBoxCalculator, it's a heavier weight machinery that attaches/detaches to the DOM tree, stateful, allocates resources to dispose etc., it didn't seem necessary here. Also, the method in the PR is safe when introducing some other text property (say, we add fontStyle or some such), because the same code is called for text styling, while the CanvasTextBBoxCalculator needs explicit passing of properties one by one. I'm interested in learning about the circumstances where CanvasTextBBoxCalculator works but a simpler approach wouldn't.

return [
{
x0: -box.actualBoundingBoxLeft + this.x - capturePad,
y0: -box.actualBoundingBoxAscent + this.y - capturePad,
x1: box.actualBoundingBoxRight + this.x + capturePad,
y1: box.actualBoundingBoxDescent + this.y + capturePad,
},
];
}

render(ctx: CanvasRenderingContext2D) {
this.setCanvasTextState(ctx);
ctx.beginPath();
ctx.fillText(this.text, this.x, this.y);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,12 @@
* Side Public License, v 1.
*/

import { TextMeasure } from '../../../../common/text_utils';
import { GoalSpec } from '../../specs';
import { Config } from '../types/config_types';
import { BulletViewModel, PickFunction, ShapeViewModel } from '../types/viewmodel_types';

/** @internal */
export function shapeViewModel(textMeasure: TextMeasure, spec: GoalSpec, config: Config): ShapeViewModel {
export function shapeViewModel(spec: GoalSpec, config: Config): ShapeViewModel {
const { width, height, margin } = config;

const innerWidth = width * (1 - Math.min(1, margin.left + margin.right));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ export function renderCanvas2d(ctx: CanvasRenderingContext2D, dpr: number, geomO
(context: CanvasRenderingContext2D) => clearCanvas(context, 200000, 200000),

(context: CanvasRenderingContext2D) =>
withContext(context, (ctx) => {
geomObjects.forEach((obj) => withContext(ctx, (ctx) => obj.render(ctx)));
}),
withContext(context, (ctx) => geomObjects.forEach((obj) => withContext(ctx, (ctx) => obj.render(ctx)))),
]);
});
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import React, { MouseEvent, RefObject } from 'react';
import { connect } from 'react-redux';
import { bindActionCreators, Dispatch } from 'redux';

import { Rectangle } from '../../../../common/geometry';
import { GoalSemanticDescription, ScreenReaderSummary } from '../../../../components/accessibility';
import { onChartRendered } from '../../../../state/actions/chart';
import { GlobalChartState } from '../../../../state/chart_state';
Expand All @@ -21,9 +22,10 @@ import {
import { getInternalIsInitializedSelector, InitStatus } from '../../../../state/selectors/get_internal_is_intialized';
import { Dimensions } from '../../../../utils/dimensions';
import { BandViewModel, nullShapeViewModel, ShapeViewModel } from '../../layout/types/viewmodel_types';
import { Mark } from '../../layout/viewmodel/geoms';
import { initialBoundingBox, Mark } from '../../layout/viewmodel/geoms';
import { geometries, getPrimitiveGeoms } from '../../state/selectors/geometries';
import { getFirstTickValueSelector, getGoalChartSemanticDataSelector } from '../../state/selectors/get_goal_chart_data';
import { getCaptureBoundingBox } from '../../state/selectors/picked_shapes';
import { renderCanvas2d } from './canvas_renderers';

interface ReactiveChartStateProps {
Expand All @@ -34,6 +36,7 @@ interface ReactiveChartStateProps {
a11ySettings: A11ySettings;
bandLabels: BandViewModel[];
firstValue: number;
captureBoundingBox: Rectangle;
}

interface ReactiveChartDispatchProps {
Expand Down Expand Up @@ -89,16 +92,19 @@ class Component extends React.Component<Props> {
chartContainerDimensions: { width, height },
forwardStageRef,
geometries,
captureBoundingBox: capture,
} = this.props;
if (!forwardStageRef.current || !this.ctx || !initialized || width === 0 || height === 0) {
return;
}
const picker = geometries.pickQuads;
const box = forwardStageRef.current.getBoundingClientRect();
const { chartCenter } = geometries;
const x = e.clientX - box.left - chartCenter.x;
const y = e.clientY - box.top - chartCenter.y;
return picker(x, y);
const x = e.clientX - box.left;
const y = e.clientY - box.top;
if (capture.x0 <= x && x <= capture.x1 && capture.y0 <= y && y <= capture.y1) {
return picker(x - chartCenter.x, y - chartCenter.y);
}
}

render() {
Expand Down Expand Up @@ -168,6 +174,7 @@ const DEFAULT_PROPS: ReactiveChartStateProps = {
a11ySettings: DEFAULT_A11Y_SETTINGS,
bandLabels: [],
firstValue: 0,
captureBoundingBox: initialBoundingBox(),
};

const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => {
Expand All @@ -182,6 +189,7 @@ const mapStateToProps = (state: GlobalChartState): ReactiveChartStateProps => {
bandLabels: getGoalChartSemanticDataSelector(state),
firstValue: getFirstTickValueSelector(state),
geoms: getPrimitiveGeoms(state),
captureBoundingBox: getCaptureBoundingBox(state),
};
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,33 +6,61 @@
* Side Public License, v 1.
*/

import { Rectangle } from '../../../../common/geometry';
import { LayerValue } from '../../../../specs';
import { GlobalChartState } from '../../../../state/chart_state';
import { createCustomCachedSelector } from '../../../../state/create_selector';
import { BulletViewModel } from '../../layout/types/viewmodel_types';
import { geometries } from './geometries';
import { initialBoundingBox, Mark } from '../../layout/viewmodel/geoms';
import { geometries, getPrimitiveGeoms } from './geometries';

function getCurrentPointerPosition(state: GlobalChartState) {
return state.interactions.pointer.current.position;
}

function fullBoundingBox(ctx: CanvasRenderingContext2D | null, geoms: Mark[]) {
const box = initialBoundingBox();
if (ctx) {
for (const g of geoms) {
for (const { x0, y0, x1, y1 } of g.boundingBoxes(ctx)) {
box.x0 = Math.min(box.x0, x0, x1);
box.y0 = Math.min(box.y0, y0, y1);
box.x1 = Math.max(box.x1, x0, x1);
box.y1 = Math.max(box.y1, y0, y1);
}
}
}
return box;
}

/** @internal */
export const getCaptureBoundingBox = createCustomCachedSelector(
[getPrimitiveGeoms],
(geoms): Rectangle => {
const textMeasurer = document.createElement('canvas');
const ctx = textMeasurer.getContext('2d');
return fullBoundingBox(ctx, geoms);
},
);

/** @internal */
export const getPickedShapes = createCustomCachedSelector(
[geometries, getCurrentPointerPosition],
(geoms, pointerPosition): BulletViewModel[] => {
[geometries, getCurrentPointerPosition, getCaptureBoundingBox],
(geoms, pointerPosition, capture): BulletViewModel[] => {
const picker = geoms.pickQuads;
const { chartCenter } = geoms;
const x = pointerPosition.x - chartCenter.x;
const y = pointerPosition.y - chartCenter.y;
return picker(x, y);
const { x, y } = pointerPosition;
return capture.x0 <= x && x <= capture.x1 && capture.y0 <= y && y <= capture.y1
? picker(x - chartCenter.x, y - chartCenter.y)
: [];
},
);

/** @internal */
export const getPickedShapesLayerValues = createCustomCachedSelector(
[getPickedShapes],
(pickedShapes): Array<Array<LayerValue>> => {
const elements = pickedShapes.map<Array<LayerValue>>((model) => {
return pickedShapes.map<Array<LayerValue>>((model) => {
const values: Array<LayerValue> = [];
values.push({
smAccessorValue: '',
Expand All @@ -44,6 +72,5 @@ export const getPickedShapesLayerValues = createCustomCachedSelector(
});
return values.reverse();
});
return elements;
},
);
Original file line number Diff line number Diff line change
Expand Up @@ -6,25 +6,19 @@
* Side Public License, v 1.
*/

import { measureText } from '../../../../common/text_utils';
import { mergePartial, RecursivePartial } from '../../../../utils/common';
import { Dimensions } from '../../../../utils/dimensions';
import { config as defaultConfig } from '../../layout/config/config';
import { Config } from '../../layout/types/config_types';
import { ShapeViewModel, nullShapeViewModel } from '../../layout/types/viewmodel_types';
import { ShapeViewModel } from '../../layout/types/viewmodel_types';
import { shapeViewModel } from '../../layout/viewmodel/viewmodel';
import { GoalSpec } from '../../specs';

/** @internal */
export function render(spec: GoalSpec, parentDimensions: Dimensions): ShapeViewModel {
const { width, height } = parentDimensions;
const { config: specConfig } = spec;
const textMeasurer = document.createElement('canvas');
const textMeasurerCtx = textMeasurer.getContext('2d');
const partialConfig: RecursivePartial<Config> = { ...specConfig, width, height };
const config: Config = mergePartial(defaultConfig, partialConfig, { mergeOptionalPartialValues: true });
if (!textMeasurerCtx) {
return nullShapeViewModel(config, { x: width / 2, y: height / 2 });
}
return shapeViewModel(measureText(textMeasurerCtx), spec, config);
return shapeViewModel(spec, config);
}
Loading