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(tooltip): improve positioning with popperjs #651

Merged
merged 52 commits into from
May 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
5ee6ad0
fix: tooltip container scroll issue
nickofthyme Apr 23, 2020
8a18b7a
Merge branch 'master' into fix/tooltip-scroll
nickofthyme Apr 23, 2020
072b09f
fix: type error in dist
nickofthyme Apr 23, 2020
0140bd2
fix: possibly fix the scrollbar in all charts
markov00 Apr 24, 2020
f574e6d
refactor: improve tooltip postioning logic
nickofthyme Apr 27, 2020
5e09582
cleanup: remove highlight type, update popper
nickofthyme Apr 28, 2020
83ef7e4
Merge branch 'master' into feat/popper-poc
nickofthyme Apr 29, 2020
f91b9cf
refactor: tooltip portal to functional component
nickofthyme Apr 29, 2020
b379f99
refactor: decouple tooltip and portal logic
nickofthyme Apr 29, 2020
547dab4
refactor: move portal into seperate folder and cleanup types
nickofthyme Apr 29, 2020
e0fecf1
feat: expand tooltip placement options
nickofthyme Apr 29, 2020
24892d1
refactor: combine real anchor and invisble anchor ref into single prop
nickofthyme Apr 29, 2020
618148e
refactor: move annotations, pull out rect and line tooltip render com…
nickofthyme Apr 29, 2020
e07649a
refactor: line and rect annotations to use Portal
nickofthyme Apr 30, 2020
3b30486
fix: line tooltip annotation position
nickofthyme Apr 30, 2020
87a57b4
fix: final cleanup of code before ready for review
nickofthyme Apr 30, 2020
4184fcb
fix: use full version of popper until library is using babel for tree…
nickofthyme Apr 30, 2020
df7675f
fix: cleanup debug styles in portal
nickofthyme Apr 30, 2020
c3b31dc
Merge branch 'master' into feat/popper-poc
nickofthyme Apr 30, 2020
73f8495
feat: add fallback knobs
nickofthyme Apr 30, 2020
a0b7cd4
fix: settings error
nickofthyme Apr 30, 2020
e64700a
fix: tooltip render on partition charts
nickofthyme Apr 30, 2020
842b11b
fix: variable naming
nickofthyme Apr 30, 2020
afc7227
test: update annotations utils to use non-projected cursor
nickofthyme Apr 30, 2020
5227c93
ci: enforce no .only in tests on ci
nickofthyme Apr 30, 2020
9d2deb5
test: update vrt
nickofthyme Apr 30, 2020
a43f7d8
test: update vrt screenshots based on changes
nickofthyme Apr 30, 2020
6261e6c
fix: multiple charts issue and scrolling issues
nickofthyme May 4, 2020
ccfe32e
fix: prevent popper updates when tooltip is not visible
nickofthyme May 4, 2020
dc5a01e
Merge branch 'master' into feat/popper-poc
nickofthyme May 4, 2020
8094278
fix: scss and typings build issues
nickofthyme May 4, 2020
7467d2a
test: update vrt to better test tooltip placement and rotations
nickofthyme May 5, 2020
884aacb
Merge branch 'master' into feat/popper-poc
nickofthyme May 5, 2020
d76de27
Merge branch 'master' into feat/popper-poc
nickofthyme May 5, 2020
eeb574f
test: fix missed vrt update
nickofthyme May 5, 2020
a6f5ded
Merge branch 'master' into feat/popper-poc
monfera May 19, 2020
26954a8
test: revert changes to vrt, add to seperate pr
nickofthyme May 26, 2020
a38ada2
refactor: rename portal to tooltip portal
nickofthyme May 26, 2020
e7256b6
refactor: cleanup hook logic
nickofthyme May 26, 2020
95def5f
Merge branch 'master' into feat/popper-poc
nickofthyme May 26, 2020
ddc44cb
fix: remove forced non-null
nickofthyme May 26, 2020
9d43a4b
fix: remove cruft from highlight tooltip type
nickofthyme May 26, 2020
c517407
test: remove all stale screenshots
nickofthyme May 26, 2020
37bd0b8
test: add back missing screenshots
nickofthyme May 26, 2020
0b365c6
docs: update api docs
nickofthyme May 26, 2020
7060feb
test: update screenshots
nickofthyme May 26, 2020
7efd73f
fix: variable naming in playground
nickofthyme May 27, 2020
1c3b868
Merge branch 'master' into feat/popper-poc
monfera May 27, 2020
9c534e7
fix: broken knobs, add rotation based defaults for popper placement
nickofthyme May 27, 2020
2484f23
docs: fix api-extractor warnings for tooltips
markov00 May 28, 2020
492b426
docs: fix api-extractor warnings for placements
markov00 May 28, 2020
514d8ac
Merge branch 'master' into feat/popper-poc
nickofthyme May 28, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
The table of contents is too big for display.
Diff view
Diff view
  •  
  •  
  •  
23 changes: 20 additions & 3 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ module.exports = {
'plugin:prettier/recommended',
'plugin:react/recommended',
],
plugins: ['@typescript-eslint', 'import', 'jest', 'unicorn', 'file-header'],
plugins: ['@typescript-eslint', 'import', 'jest', 'unicorn', 'file-header', 'react-hooks'],

env: {
es6: true,
Expand Down Expand Up @@ -104,6 +104,8 @@ module.exports = {
'block',
['-\\*-(.*)-\\*-', 'eslint(.*)', '@jest-environment'],
],
'react-hooks/rules-of-hooks': 'error',
'react-hooks/exhaustive-deps': 'warn',
},
settings: {
'import/resolver': {
Expand All @@ -127,11 +129,26 @@ module.exports = {
files: ['stories/**/*.tsx', 'stories/**/*.ts', '*.test.ts', '*.test.tsx'],
rules: {
'no-restricted-properties': [
2,
process.env.NODE_ENV === 'production' ? 2 : 1,
{
object: 'Math',
property: 'random',
message: 'Please use the `getRandomNumber` to create seeded random function in `stories/` and `tests/`',
message: 'Please use the `getRandomNumber` to create seeded random function in `stories/` and `tests/`.',
},
{
object: 'describe',
property: 'only',
message: 'Please remove before committing changes.',
},
{
object: 'it',
property: 'only',
message: 'Please remove before committing changes.',
},
{
object: 'test',
property: 'only',
message: 'Please remove before committing changes.',
nickofthyme marked this conversation as resolved.
Show resolved Hide resolved
},
],
},
Expand Down
37 changes: 33 additions & 4 deletions .playground/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,20 +9,49 @@
html,
body {
background: blanchedalmond !important;
/*margin-left: 8px !important;*/
/*padding: 8px !important;*/
/*height: 100%;*/
/*width: 2000px;
}
#root {
position: absolute;
/*
top: 0;
left: 0;
*/
/* width: 100%;
height: 100%;*/
/* overflow-x: hidden; */
}
.chart {
background: white;
width: 800px;
height: 300px;
margin: 20px;
/*display: inline-block;
position: relative;
*/
width: 100%;
height: 500px;
overflow: auto;
}

.testing {
background: aquamarine;
position: relative;
width: 100%;
overflow: auto;
}
.page {
padding: 100px;
}
label {
display: block;
}
</style>
</head>
<body>
<div id="root"></div>
<div class="page">
<div id="root"></div>
</div>
<script src="bundle.js" type="text/javascript"></script>
</body>
</html>
36 changes: 2 additions & 34 deletions .playground/playground.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,45 +17,13 @@
* under the License. */

import React from 'react';
import { Chart, Axis, Position, Settings, AreaSeries, ScaleType, DataGenerator } from '../src';
import { getRandomNumberGenerator } from '../src/mocks/utils';
import { Example } from '../stories/treemap/6_custom_style';

const dg = new DataGenerator(500, getRandomNumberGenerator());
const basicData = dg.generateBasicSeries();
export class Playground extends React.Component {
state = {
data: basicData,
};
onBrushEnd = () => {
this.setState({ data: [] });
setTimeout(() => {
this.setState({
data: dg.generateBasicSeries(),
});
}, 100);
};
render() {
return (
<div className="testing">
<div className="chart">
<Chart className="story-chart">
<Settings onBrushEnd={this.onBrushEnd} />
<Axis id="bottom" position={Position.Bottom} title="bottom" showOverlappingTicks={true} />
<Axis id="left" title="left" position={Position.Left} tickFormat={(d) => Number(d).toFixed(2)} />
<Axis id="top" position={Position.Top} title="top" showOverlappingTicks={true} />
<Axis id="right" title="right" position={Position.Right} tickFormat={(d) => Number(d).toFixed(2)} />
{this.state.data.length > 0 && (
<AreaSeries
id="lines"
xScaleType={ScaleType.Linear}
yScaleType={ScaleType.Linear}
xAccessor="x"
yAccessors={['y']}
data={this.state.data}
/>
)}
</Chart>
</div>
<div className="chart">{Example()}</div>
</div>
);
}
Expand Down
77 changes: 50 additions & 27 deletions api/charts.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
```ts

import { $Values } from 'utility-types';
import { ComponentType } from 'react';
import React from 'react';

// @public
Expand Down Expand Up @@ -403,6 +404,9 @@ export const CurveType: Readonly<{
// @public (undocumented)
export type CurveType = $Values<typeof CurveType>;

// @public
export type CustomTooltip = ComponentType<TooltipInfo>;

// Warning: (ae-missing-release-tag) "DARK_THEME" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
Expand Down Expand Up @@ -493,14 +497,10 @@ export const DEFAULT_MISSING_COLOR = "red";
// @public (undocumented)
export const DEFAULT_SETTINGS_SPEC: SettingsSpec;

// Warning: (ae-missing-release-tag) "DEFAULT_TOOLTIP_SNAP" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
// @public
export const DEFAULT_TOOLTIP_SNAP = true;

// Warning: (ae-missing-release-tag) "DEFAULT_TOOLTIP_TYPE" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
// @public
export const DEFAULT_TOOLTIP_TYPE: "vertical";

// Warning: (ae-missing-release-tag) "DefaultSettingsProps" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
Expand Down Expand Up @@ -678,9 +678,9 @@ export type HistogramModeAlignment = 'start' | 'center' | 'end';
//
// @public (undocumented)
export const HistogramModeAlignments: Readonly<{
Start: HistogramModeAlignment;
Center: HistogramModeAlignment;
End: HistogramModeAlignment;
Start: LineAlignSetting;
Center: LineAlignSetting;
End: LineAlignSetting;
Comment on lines +681 to +683
Copy link
Member

Choose a reason for hiding this comment

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

weird....I will open a PR to fix this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, wasn't sure what this was from.

}>;

// Warning: (ae-forgotten-export) The symbol "BinaryAccessorFn" needs to be exported by the entry point index.d.ts
Expand Down Expand Up @@ -931,6 +931,28 @@ export const PartitionLayout: Readonly<{
// @public (undocumented)
export type PartitionLayout = $Values<typeof PartitionLayout>;

// @public
export const Placement: Readonly<{
Top: "top";
Bottom: "bottom";
Left: "left";
Right: "right";
TopStart: "top-start";
TopEnd: "top-end";
BottomStart: "bottom-start";
BottomEnd: "bottom-end";
RightStart: "right-start";
RightEnd: "right-end";
LeftStart: "left-start";
LeftEnd: "left-end";
Auto: "auto";
AutoStart: "auto-start";
AutoEnd: "auto-end";
}>;

// @public
export type Placement = $Values<typeof Placement>;

// Warning: (ae-missing-release-tag) "PointerEvent" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
Expand Down Expand Up @@ -1296,7 +1318,7 @@ export interface SettingsSpec extends Spec {
showLegend: boolean;
showLegendExtra: boolean;
theme?: PartialTheme | PartialTheme[];
tooltip: TooltipType | TooltipProps;
tooltip: TooltipSettings;
// Warning: (ae-forgotten-export) The symbol "Domain" needs to be exported by the entry point index.d.ts
//
// (undocumented)
Expand Down Expand Up @@ -1428,23 +1450,28 @@ export type TickStyle = StrokeStyle & Visible;
// @public (undocumented)
export function timeFormatter(format: string): TickFormatter;

// Warning: (ae-missing-release-tag) "TooltipProps" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
// @public
export interface TooltipInfo {
header: TooltipValue | null;
values: TooltipValue[];
}

// @public
export interface TooltipProps {
// (undocumented)
boundary?: HTMLElement | 'chart';
customTooltip?: CustomTooltip;
fallbackPlacements?: Placement[];
headerFormatter?: TooltipValueFormatter;
// (undocumented)
placement?: Placement;
snap?: boolean;
// (undocumented)
type?: TooltipType;
// (undocumented)
// @alpha
unit?: string;
}

// Warning: (ae-missing-release-tag) "TooltipType" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
// Warning: (ae-missing-release-tag) "TooltipType" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public
export type TooltipSettings = TooltipType | TooltipProps;

// @public
export const TooltipType: Readonly<{
VerticalCursor: "vertical";
Expand All @@ -1453,12 +1480,10 @@ export const TooltipType: Readonly<{
None: "none";
}>;

// @public (undocumented)
// @public
export type TooltipType = $Values<typeof TooltipType>;

// Warning: (ae-missing-release-tag) "TooltipValue" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
// @public
export interface TooltipValue {
color: Color;
isHighlighted: boolean;
Expand All @@ -1470,9 +1495,7 @@ export interface TooltipValue {
valueAccessor?: Accessor;
}

// Warning: (ae-missing-release-tag) "TooltipValueFormatter" is exported by the package, but it is missing a release tag (@alpha, @beta, @public, or @internal)
//
// @public (undocumented)
// @public
export type TooltipValueFormatter = (data: TooltipValue) => JSX.Element | string;

// @public
Expand Down
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
2 changes: 2 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@
"eslint-plugin-jest": "^23.0.4",
"eslint-plugin-prettier": "^3.1.2",
"eslint-plugin-react": "^7.19.0",
"eslint-plugin-react-hooks": "^4.0.3",
"eslint-plugin-unicorn": "^17.2.0",
"geckodriver": "^1.19.1",
"husky": "^3.1.0",
Expand Down Expand Up @@ -168,6 +169,7 @@
"webpack-dev-server": "^3.3.1"
},
"dependencies": {
"@popperjs/core": "^2.4.0",
"classnames": "^2.2.6",
"d3-array": "^1.2.4",
"d3-collection": "^1.0.7",
Expand Down
4 changes: 2 additions & 2 deletions src/chart_types/partition_chart/state/chart_state.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import { computeLegendSelector } from './selectors/compute_legend';
import { getLegendItemsLabels } from './selectors/get_legend_items_labels';
import { HighlighterFromHover } from '../renderer/dom/highlighter_hover';
import { HighlighterFromLegend } from '../renderer/dom/highlighter_legend';
import { getPieSpecOrNull } from './selectors/pie_spec';
import { getPieSpec } from './selectors/pie_spec';

const EMPTY_MAP = new Map();

Expand All @@ -47,7 +47,7 @@ export class PartitionState implements InternalChartState {
}
chartType = ChartTypes.Partition;
isInitialized(globalState: GlobalChartState) {
return globalState.specsInitialized && getPieSpecOrNull(globalState) !== null;
return globalState.specsInitialized && getPieSpec(globalState) !== null;
}
isBrushAvailable() {
return false;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import createCachedSelector from 're-reselect';
import { LegendItem } from '../../../../commons/legend';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { getPieSpecOrNull } from './pie_spec';
import { getPieSpec } from './pie_spec';
import { partitionGeometries } from './geometries';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { PrimitiveValue } from '../../layout/utils/group_by_rollup';
Expand All @@ -29,7 +29,7 @@ import { Position } from '../../../../utils/commons';

/** @internal */
export const computeLegendSelector = createCachedSelector(
[getPieSpecOrNull, getSettingsSpecSelector, partitionGeometries, getFlatHierarchy],
[getPieSpec, getSettingsSpecSelector, partitionGeometries, getFlatHierarchy],
(pieSpec, settings, geoms, sortedItems): LegendItem[] => {
if (!pieSpec) {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import createCachedSelector from 're-reselect';
import { getTree } from './tree';
import { getChartIdSelector } from '../../../../state/selectors/get_chart_id';
import { getPieSpecOrNull } from './pie_spec';
import { getPieSpec } from './pie_spec';
import { HierarchyOfArrays, CHILDREN_KEY } from '../../layout/utils/group_by_rollup';
import { Layer } from '../../specs';
import { LegendItemLabel } from '../../../../state/selectors/get_legend_items_labels';
Expand All @@ -28,7 +28,7 @@ import { SettingsSpec } from '../../../../specs';

/** @internal */
export const getLegendItemsLabels = createCachedSelector(
[getPieSpecOrNull, getSettingsSpecSelector, getTree],
[getPieSpec, getSettingsSpecSelector, getTree],
(pieSpec, { legendMaxDepth }, tree): LegendItemLabel[] => {
if (!pieSpec) {
return [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { GlobalChartState, PointerState } from '../../../../state/chart_state';
import { getSettingsSpecSelector } from '../../../../state/selectors/get_settings_specs';
import { SettingsSpec, LayerValue } from '../../../../specs';
import { getPickedShapesLayerValues } from './picked_shapes';
import { getPieSpecOrNull } from './pie_spec';
import { getPieSpec } from './pie_spec';
import { ChartTypes } from '../../..';
import { SeriesIdentifier } from '../../../../commons/series_id';
import { isClicking } from '../../../../state/utils';
Expand All @@ -41,7 +41,7 @@ export function createOnElementClickCaller(): (state: GlobalChartState) => void
return (state: GlobalChartState) => {
if (selector === null && state.chartType === ChartTypes.Partition) {
selector = createCachedSelector(
[getPieSpecOrNull, getLastClickSelector, getSettingsSpecSelector, getPickedShapesLayerValues],
[getPieSpec, getLastClickSelector, getSettingsSpecSelector, getPickedShapesLayerValues],
(pieSpec, lastClick: PointerState | null, settings: SettingsSpec, pickedShapes): void => {
if (!pieSpec) {
return;
Expand Down
Loading