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(legend): allow color picker component #545

Merged
merged 24 commits into from
Feb 27, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
274a2de
feat: add color picker render prop
nickofthyme Feb 10, 2020
ab43639
chore: remove color override global logic in favor of local state
nickofthyme Feb 11, 2020
8f0ae0e
tests: add tests for render legend
nickofthyme Feb 11, 2020
156131c
Merge branch 'master' into feat/color-picker-prop
nickofthyme Feb 12, 2020
f056b77
chore: add parsing action and split tooltipValues (#516)
markov00 Feb 13, 2020
2242fca
refactor: decouple legend and tooltip phase 1 (#491)
markov00 Feb 14, 2020
3c5e5aa
Merge branch 'master' into feat/color-picker-prop
nickofthyme Feb 18, 2020
57e5ea8
Merge branch 'master' into feat/color-picker-prop
nickofthyme Feb 21, 2020
6546553
refactor: color picker display logic
nickofthyme Feb 21, 2020
57f5f64
fix: bypass flacky legend dimensions in snapshot
nickofthyme Feb 24, 2020
5bf4016
tests: fix last test
nickofthyme Feb 24, 2020
71ee64c
fix: test snapshot
nickofthyme Feb 24, 2020
1db0634
chore: fix pr comments
nickofthyme Feb 25, 2020
89359b4
Merge branch 'master' into feat/color-picker-prop
nickofthyme Feb 25, 2020
9e6bfdb
refactor: update logic for storybook example
nickofthyme Feb 26, 2020
0cef864
Merge branch 'master' into feat/color-picker-prop
nickofthyme Feb 26, 2020
1cfb649
refactor: generic key types to be more specific
nickofthyme Feb 26, 2020
51284ff
refactor: color state management logic
nickofthyme Feb 26, 2020
4030188
chore: fix merge conflicts
nickofthyme Feb 26, 2020
310240f
tests: generalize common page object methods
nickofthyme Feb 26, 2020
e376620
test: add legend color picker interaction VRT
nickofthyme Feb 26, 2020
400a70b
Merge branch 'master' into feat/color-picker-prop
nickofthyme Feb 26, 2020
4386a19
fix: vrt flakyness and errors
nickofthyme Feb 27, 2020
5cf51e4
chore: update pr comments, repalce string types with Color
nickofthyme Feb 27, 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
1 change: 1 addition & 0 deletions .eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ module.exports = {
'@typescript-eslint/ban-ts-ignore': 'off',
'@typescript-eslint/no-inferrable-types': 'off',
'react/jsx-curly-brace-presence': ['error', { props: 'never', children: 'never' }],
'react/prop-types': 0,
},
settings: {
'import/resolver': {
Expand Down
2 changes: 2 additions & 0 deletions .storybook/style.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
@import '../node_modules/@elastic/eui/src/theme_light.scss';

.story-chart {
box-sizing: border-box;
background: white;
Expand Down
9 changes: 9 additions & 0 deletions integration/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@ module.exports = Object.assign(
'ts-jest': {
tsConfig: '<rootDir>/tsconfig.json',
},
/*
* The window and HTMLElement globals are required to use @elastic/eui with VRT
*
* The jest-puppeteer-docker env extends a node test environment and not jsdom test environment.
* Some EUI components that are included in the bundle, but not used, require the jsdom setup.
* To bypass these errors we are just mocking both as empty objects.
*/
window: {},
HTMLElement: {},
},
},
jestPuppeteerDocker,
Expand Down
165 changes: 128 additions & 37 deletions integration/page_objects/common.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,48 @@ interface ScreenshotDOMElementOptions {
path?: string;
}

type ScreenshotElementAtUrlOptions = ScreenshotDOMElementOptions & {
/**
* timeout for waiting on element to appear in DOM
*
* @default JEST_TIMEOUT
*/
timeout?: number;
/**
* any desired action to be performed after loading url, prior to screenshot
*/
action?: () => void | Promise<void>;
/**
* Selector used to wait on DOM element
*/
waitSelector?: string;
/**
* Delay to take screenshot after element is visiable
*/
delay?: number;
};

class CommonPage {
readonly chartWaitSelector = '.echChartStatus[data-ech-render-complete=true]';
readonly chartSelector = '.echChart';

/**
* Parse url from knob storybook url to iframe storybook url
*
* @param url
*/
static parseUrl(url: string): string {
const { query } = Url.parse(url);

return `${baseUrl}?${query}${query ? '&' : ''}knob-debug=false`;
}
async getBoundingClientRect(selector = '.echChart') {

/**
* Get getBoundingClientRect of selected element
*
* @param selector
*/
async getBoundingClientRect(selector: string) {
return await page.evaluate((selector) => {
const element = document.querySelector(selector);

Expand All @@ -34,12 +69,16 @@ class CommonPage {
return { left: x, top: y, width, height, id: element.id };
}, selector);
}

/**
* Capture screenshot or chart element only
* Capture screenshot of selected element only
*
* @param selector
* @param options
*/
async screenshotDOMElement(selector = '.echChart', opts?: ScreenshotDOMElementOptions) {
const padding: number = opts && opts.padding ? opts.padding : 0;
const path: string | undefined = opts && opts.path ? opts.path : undefined;
async screenshotDOMElement(selector: string, options?: ScreenshotDOMElementOptions) {
const padding: number = options && options.padding ? options.padding : 0;
const path: string | undefined = options && options.path ? options.path : undefined;
const rect = await this.getBoundingClientRect(selector);

return page.screenshot({
Expand All @@ -53,69 +92,121 @@ class CommonPage {
});
}

async moveMouseRelativeToDOMElement(mousePosition: { x: number; y: number }, selector = '.echChart') {
const chartContainer = await this.getBoundingClientRect(selector);
await page.mouse.move(chartContainer.left + mousePosition.x, chartContainer.top + mousePosition.y);
/**
* Move mouse relative to element
*
* @param mousePosition
* @param selector
*/
async moveMouseRelativeToDOMElement(mousePosition: { x: number; y: number }, selector: string) {
const element = await this.getBoundingClientRect(selector);
await page.mouse.move(element.left + mousePosition.x, element.top + mousePosition.y);
}

/**
* Click mouse relative to element
*
* @param mousePosition
* @param selector
*/
async clickMouseRelativeToDOMElement(mousePosition: { x: number; y: number }, selector: string) {
const element = await this.getBoundingClientRect(selector);
await page.mouse.click(element.left + mousePosition.x, element.top + mousePosition.y);
}

/**
* Expect a chart given a url from storybook.
* Expect an element given a url and selector from storybook
*
* - Note: No need to fix host or port. They will be set automatically.
*
* @param url Storybook url from knobs section
* @param selector selector of element to screenshot
* @param options
*/
async expectChartAtUrlToMatchScreenshot(url: string) {
async expectElementAtUrlToMatchScreenshot(
url: string,
selector: string = 'body',
options?: ScreenshotElementAtUrlOptions,
) {
try {
await this.loadChartFromURL(url);
await this.waitForElement();
await this.loadElementFromURL(url, options?.waitSelector ?? selector, options?.timeout);

const chart = await this.screenshotDOMElement();
if (options?.action) {
await options.action();
}

if (!chart) {
throw new Error(`Error: Unable to find chart element\n\n\t${url}`);
if (options?.delay) {
await page.waitFor(options.delay);
}

expect(chart).toMatchImageSnapshot();
const element = await this.screenshotDOMElement(selector, options);

if (!element) {
throw new Error(`Error: Unable to find element\n\n\t${url}`);
}

expect(element).toMatchImageSnapshot();
} catch (error) {
throw new Error(error);
}
}

/**
* Expect a chart given a url from storybook.
*
* - Note: No need to fix host or port. They will be set automatically.
* Expect a chart given a url from storybook
*
* @param url Storybook url from knobs section
* @param options
*/
async expectChartWithMouseAtUrlToMatchScreenshot(url: string, mousePosition: { x: number; y: number }) {
try {
await this.loadChartFromURL(url);
await this.waitForElement();
await this.moveMouseRelativeToDOMElement(mousePosition);
const chart = await this.screenshotDOMElement();
if (!chart) {
throw new Error(`Error: Unable to find chart element\n\n\t${url}`);
}
async expectChartAtUrlToMatchScreenshot(url: string, options?: ScreenshotElementAtUrlOptions) {
await this.expectElementAtUrlToMatchScreenshot(url, this.chartSelector, {
waitSelector: this.chartWaitSelector,
...options,
});
}

expect(chart).toMatchImageSnapshot();
} catch (error) {
throw new Error(`${error}\n\n${url}`);
}
/**
* Expect a chart given a url from storybook with mouse move
*
* @param url Storybook url from knobs section
* @param mousePosition - postion of mouse relative to chart
* @param options
*/
async expectChartWithMouseAtUrlToMatchScreenshot(
url: string,
mousePosition: { x: number; y: number },
options?: Omit<ScreenshotElementAtUrlOptions, 'action'>,
) {
const action = async () => await this.moveMouseRelativeToDOMElement(mousePosition, this.chartSelector);
await this.expectChartAtUrlToMatchScreenshot(url, {
...options,
action,
});
}
async loadChartFromURL(url: string) {

/**
* Loads storybook page from raw url, and waits for element
*
* @param url Storybook url from knobs section
* @param waitSelector selector of element to wait to appear in DOM
* @param timeout timeout for waiting on element to appear in DOM
*/
async loadElementFromURL(url: string, waitSelector?: string, timeout?: number) {
const cleanUrl = CommonPage.parseUrl(url);
await page.goto(cleanUrl);
this.waitForElement();

if (waitSelector) {
await this.waitForElement(waitSelector, timeout);
}
}

/**
* Wait for an element to be on the DOM
* @param {string} [selector] the DOM selector to wait for, default to '.echChartStatus[data-ech-render-complete=true]'
*
* @param {string} [waitSelector] the DOM selector to wait for, default to '.echChartStatus[data-ech-render-complete=true]'
* @param {number} [timeout] - the timeout for the operation, default to 10000ms
*/
async waitForElement(selector = '.echChartStatus[data-ech-render-complete=true]', timeout = JEST_TIMEOUT) {
await page.waitForSelector(selector, { timeout });
async waitForElement(waitSelector: string, timeout = JEST_TIMEOUT) {
await page.waitForSelector(waitSelector, { timeout });
}
}

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.
13 changes: 13 additions & 0 deletions integration/tests/legend_stories.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,17 @@ describe('Legend stories', () => {
'http://localhost:9001/?path=/story/legend--legend-spacing-buffer&knob-legend buffer value=0',
);
});

it('should render color picker on mouse click', async () => {
const action = async () => await common.clickMouseRelativeToDOMElement({ x: 0, y: 0 }, '.echLegendItem__color');
await common.expectElementAtUrlToMatchScreenshot(
'http://localhost:9001/?path=/story/legend--color-picker',
'body',
{
action,
waitSelector: common.chartWaitSelector,
delay: 500, // needed for popover animation to complete
},
);
});
});
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@
"@babel/preset-react": "^7.8.3",
"@commitlint/cli": "^8.1.0",
"@commitlint/config-conventional": "^8.1.0",
"@elastic/datemath": "^5.0.2",
"@elastic/eui": "^16.0.1",
"@mdx-js/loader": "^1.5.5",
"@semantic-release/changelog": "^3.0.6",
Expand Down
2 changes: 2 additions & 0 deletions scripts/setup_enzyme.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,3 +2,5 @@ import { configure } from 'enzyme';
import Adapter from 'enzyme-adapter-react-16';

configure({ adapter: new Adapter() });

process.env.RNG_SEED = 'jest-unit-tests';
markov00 marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 2 additions & 1 deletion src/chart_types/partition_chart/layout/utils/calcs.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { Ratio } from '../types/geometry_types';
import { RgbTuple, stringToRGB } from './d3_utils';
import { Color } from '../../../../utils/commons';

export function hueInterpolator(colors: RgbTuple[]) {
return (d: number) => {
Expand All @@ -26,7 +27,7 @@ export function arrayToLookup(keyFun: Function, array: Array<any>) {
return Object.assign({}, ...array.map((d) => ({ [keyFun(d)]: d })));
}

export function colorIsDark(color: string) {
export function colorIsDark(color: Color) {
// fixme this assumes a white or very light background
const rgba = stringToRGB(color);
const { r, g, b, opacity } = rgba;
Expand Down
4 changes: 2 additions & 2 deletions src/chart_types/xy_chart/annotations/annotation_utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import {
AnnotationRectProps,
computeRectAnnotationDimensions,
} from './rect_annotation_tooltip';
import { Rotation, Position } from '../../../utils/commons';
import { Rotation, Position, Color } from '../../../utils/commons';

export type AnnotationTooltipFormatter = (details?: string) => JSX.Element | null;

Expand Down Expand Up @@ -54,7 +54,7 @@ export interface AnnotationMarker {
icon: JSX.Element;
position: { top: number; left: number };
dimension: { width: number; height: number };
color: string;
color: Color;
}

export type AnnotationDimensions = AnnotationLineProps[] | AnnotationRectProps[];
Expand Down
15 changes: 8 additions & 7 deletions src/chart_types/xy_chart/legend/legend.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import { getAxesSpecForSpecId, LastValues, getSpecsById } from '../state/utils';
import { identity } from '../../../utils/commons';
import { identity, Color } from '../../../utils/commons';
import {
SeriesCollectionValue,
getSeriesIndex,
getSortedDataSeriesColorsValuesMap,
getSeriesName,
XYChartSeriesIdentifier,
SeriesKey,
} from '../utils/series';
import { AxisSpec, BasicSeriesSpec, Postfixes, isAreaSeriesSpec, isBarSeriesSpec } from '../utils/specs';
import { Y0_ACCESSOR_POSTFIX, Y1_ACCESSOR_POSTFIX } from '../tooltip/tooltip';
Expand All @@ -17,8 +18,8 @@ interface FormattedLastValues {
}

export type LegendItem = Postfixes & {
key: string;
color: string;
key: SeriesKey;
color: Color;
name: string;
seriesIdentifier: XYChartSeriesIdentifier;
isSeriesVisible?: boolean;
Expand Down Expand Up @@ -54,14 +55,14 @@ export function getItemLabel(
}

export function computeLegend(
seriesCollection: Map<string, SeriesCollectionValue>,
seriesColors: Map<string, string>,
seriesCollection: Map<SeriesKey, SeriesCollectionValue>,
seriesColors: Map<SeriesKey, Color>,
specs: BasicSeriesSpec[],
defaultColor: string,
axesSpecs: AxisSpec[],
deselectedDataSeries: XYChartSeriesIdentifier[] = [],
): Map<string, LegendItem> {
const legendItems: Map<string, LegendItem> = new Map();
): Map<SeriesKey, LegendItem> {
const legendItems: Map<SeriesKey, LegendItem> = new Map();
const sortedCollection = getSortedDataSeriesColorsValuesMap(seriesCollection);

sortedCollection.forEach((series, key) => {
Expand Down
2 changes: 1 addition & 1 deletion src/chart_types/xy_chart/renderer/canvas/axes/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export interface AxesProps {
axesVisibleTicks: Map<AxisId, AxisTick[]>;
axesSpecs: AxisSpec[];
axesTicksDimensions: Map<AxisId, AxisTicksDimensions>;
axesPositions: Map<string, Dimensions>;
axesPositions: Map<AxisId, Dimensions>;
axisStyle: AxisConfig;
debug: boolean;
chartDimensions: Dimensions;
Expand Down
Loading