Skip to content

Commit

Permalink
fix(charts): remove cheerio, use native DOM methods for tooltip manip…
Browse files Browse the repository at this point in the history 
…ulation
  • Loading branch information
@tay1orjones
tay1orjones committed Oct 26, 2020
1 parent aba1ef5 commit 45f703b
Show file tree
Hide file tree
Showing 7 changed files with 144 additions and 396 deletions.
4 changes: 0 additions & 4 deletions package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@
"carbon-components": "10.17.0",
"carbon-components-react": "7.17.0",
"carbon-icons": "^7.0.7",
"cheerio": "^1.0.0-rc.3",
"classnames": "^2.2.5",
"core-js": "3.6.5",
"downshift": "5.2.1",
Expand All @@ -153,7 +152,6 @@
"react-sizeme": "^2.6.3",
"react-transition-group": "^2.6.0",
"react-visibility-sensor": "^5.0.2",
"rollup-plugin-auto-external": "^2.0.0",
"styled-components": "^4.1.3",
"use-deep-compare-effect": "^1.2.0",
"use-lang-direction": "^0.1.11",
Expand Down Expand Up @@ -265,8 +263,6 @@
"rollup-plugin-copy": "^3.2.0",
"rollup-plugin-filesize": "^6.0.0",
"rollup-plugin-json": "^4.0.0",
"rollup-plugin-node-builtins": "^2.1.2",
"rollup-plugin-node-globals": "^1.4.0",
"rollup-plugin-node-resolve": "^4.0.0",
"rollup-plugin-postcss": "^2.0.3",
"rollup-plugin-replace": "^2.1.0",
Expand Down
95 changes: 45 additions & 50 deletions rollup.config.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import postcss from 'rollup-plugin-postcss';
import copy from 'rollup-plugin-copy';
import autoprefixer from 'autoprefixer';
import json from 'rollup-plugin-json';
import builtins from 'rollup-plugin-node-builtins';
import svgr from '@svgr/rollup';

const packageJson = require('./package.json');
Expand All @@ -30,51 +29,6 @@ const external = (id) => {
id.includes('@babel/runtime')
);
};
const plugins = [
resolve({ mainFields: ['module', 'main'], extensions, preferBuiltins: true }),
builtins(),
commonjs({
namedExports: {
'react/index.js': [
'Children',
'Component',
'PureComponent',
'Fragment',
'PropTypes',
'createElement',
],
'react-dom/index.js': ['render'],
'react-is/index.js': ['isForwardRef'],
'core-js': 'CoreJs',
},

include: '/node_modules/',
}),

babel({
exclude: 'node_modules/**',
runtimeHelpers: true,
}),
replace({
'process.env.NODE_ENV': JSON.stringify(env),
}),
json({
// All JSON files will be parsed by default,
// but you can also specifically include/exclude files
exclude: ['node_modules'],
// for tree-shaking, properties will be declared as
// variables, using either `var` or `const`
preferConst: true, // Default: false
// specify indentation for the generated default export —
// defaults to '\t'
indent: ' ',
// ignores indent and generates the smallest code
compact: true, // Default: false
// generate a named export for every property of the JSON object
namedExports: true, // Default: true
}),
svgr(),
];

export default [
// CommonJS & ESM
Expand All @@ -93,7 +47,50 @@ export default [
},
],
external,
plugins: [...plugins, ...prodSettings],
plugins: [
resolve({ mainFields: ['module', 'main'], extensions }),
commonjs({
namedExports: {
'react/index.js': [
'Children',
'Component',
'PureComponent',
'Fragment',
'PropTypes',
'createElement',
],
'react-dom/index.js': ['render'],
'react-is/index.js': ['isForwardRef'],
'core-js': 'CoreJs',
},

include: '/node_modules/',
}),
babel({
exclude: 'node_modules/**',
runtimeHelpers: true,
}),
replace({
'process.env.NODE_ENV': JSON.stringify(env),
}),
json({
// All JSON files will be parsed by default,
// but you can also specifically include/exclude files
exclude: ['node_modules'],
// for tree-shaking, properties will be declared as
// variables, using either `var` or `const`
preferConst: true, // Default: false
// specify indentation for the generated default export —
// defaults to '\t'
indent: ' ',
// ignores indent and generates the smallest code
compact: true, // Default: false
// generate a named export for every property of the JSON object
namedExports: true, // Default: true
}),
svgr(),
...prodSettings,
],
},
// Compile styles
{
Expand Down Expand Up @@ -149,10 +146,8 @@ export default [
plugins: [
resolve({
browser: true,
extensions: ['.mjs', '.js', '.jsx', '.json'],
preferBuiltins: true,
extensions,
}),
builtins(),
commonjs({
namedExports: {
'react-js': ['isValidElementType'],
Expand Down
14 changes: 9 additions & 5 deletions src/components/BarChartCard/barChartUtils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,13 @@ import moment from 'moment';
import isNil from 'lodash/isNil';
import isEmpty from 'lodash/isEmpty';
import capitalize from 'lodash/capitalize';
import cheerio from 'cheerio';
import { blue, cyan, green, magenta, purple, red, teal } from '@carbon/colors';

import {
BAR_CHART_TYPES,
BAR_CHART_LAYOUTS,
} from '../../constants/LayoutConstants';
import { convertStringsToDOMElement } from '../../utils/componentUtilityFunctions';

/**
* Generate fake, sample values for isEditable preview state
Expand Down Expand Up @@ -335,7 +335,7 @@ export const handleTooltip = (
: dataOrHoveredElement;
const typedData = Array.isArray(data) ? data[0] : data;

const parsedTooltip = cheerio.load(defaultTooltip);
const [parsedTooltip] = convertStringsToDOMElement([defaultTooltip]);

// If theres a time attribute, add an extra list item with the formatted date
if (timeDataSourceId) {
Expand All @@ -351,12 +351,16 @@ export const handleTooltip = (
</li>`
: '';

const [parsedDateLabel] = convertStringsToDOMElement([dateLabel]);

// First remove carbon charts default Date tooltip
// the first <li> will always be carbon chart's Dates row in this case
parsedTooltip('li:first-child').replaceWith(dateLabel);
// the first <li> will always be carbon chart's Dates row in this case, replace with our date format <li>
parsedTooltip
.querySelector('li:first-child')
.replaceWith(parsedDateLabel.querySelector('li'));
}

return parsedTooltip.html('ul');
return parsedTooltip.querySelector('ul').outerHTML;
};

/**
Expand Down
49 changes: 34 additions & 15 deletions src/components/TimeSeriesCard/TimeSeriesCard.jsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,11 @@ import filter from 'lodash/filter';
import memoize from 'lodash/memoize';
import capitalize from 'lodash/capitalize';
import useDeepCompareEffect from 'use-deep-compare-effect';
import cheerio from 'cheerio';

import { csvDownloadHandler } from '../../utils/componentUtilityFunctions';
import {
convertStringsToDOMElement,
csvDownloadHandler,
} from '../../utils/componentUtilityFunctions';
import { CardPropTypes, ZoomBarPropTypes } from '../../constants/CardPropTypes';
import {
CARD_SIZES,
Expand Down Expand Up @@ -182,12 +184,13 @@ const memoizedGenerateSampleValues = memoize(generateSampleValues);

/**
* Extends default tooltip with the additional date information, and optionally alert information
* @param {object} data data object for this particular datapoint should have a date field containing the timestamp
* @param {object} dataOrHoveredElement data object for this particular datapoint should have a date field containing the timestamp
* @param {string} defaultTooltip Default HTML generated for this tooltip that needs to be marked up
* @param {array} alertRanges Array of alert range information to search
* @param {string} alertDetected Translated string to indicate that the alert is detected
* @param {bool} showTimeInGMT
* @param {string} tooltipDataFormatPattern
* @param {string} tooltipDateFormatPattern
* @returns {string} DOM representation of the tooltip
*/
export const handleTooltip = (
dataOrHoveredElement,
Expand All @@ -205,11 +208,11 @@ export const handleTooltip = (
: data?.date?.getTime();
const dateLabel = timeStamp
? `<li class='datapoint-tooltip'>
<p class='label'>${(showTimeInGMT // show timestamp in gmt or local time
? moment.utc(timeStamp)
: moment(timeStamp)
).format(tooltipDateFormatPattern)}</p>
</li>`
<p class='label'>${(showTimeInGMT // show timestamp in gmt or local time
? moment.utc(timeStamp)
: moment(timeStamp)
).format(tooltipDateFormatPattern)}</p>
</li>`
: '';
const matchingAlertRanges = findMatchingAlertRange(alertRanges, data);
const matchingAlertLabels = Array.isArray(matchingAlertRanges)
Expand All @@ -220,13 +223,29 @@ export const handleTooltip = (
)
.join('')
: '';
const parsedTooltip = cheerio.load(defaultTooltip);
// the first <li> will always be carbon chart's Dates row in this case, replace with our date format
parsedTooltip('li:first-child').replaceWith(dateLabel);

// append the matching alert labels
parsedTooltip('ul').append(matchingAlertLabels);
return parsedTooltip.html('ul');
// Convert strings to DOM Elements so we can easily reason about them and manipulate/replace pieces.
const [
defaultTooltipDOM,
dateLabelDOM,
matchingAlertLabelsDOM,
] = convertStringsToDOMElement([
defaultTooltip,
dateLabel,
matchingAlertLabels,
]);

// The first <li> will always be carbon chart's Dates row in this case, replace with our date format <li>
defaultTooltipDOM
.querySelector('li:first-child')
.replaceWith(dateLabelDOM.querySelector('li'));

// Append all the matching alert labels
matchingAlertLabelsDOM.querySelectorAll('li').forEach((label) => {
defaultTooltipDOM.append(label);
});

return defaultTooltipDOM.querySelector('ul').outerHTML;
};

/**
Expand Down
32 changes: 32 additions & 0 deletions src/utils/__tests__/componentUtilityFunctions.test.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import {
canFit,
filterValidAttributes,
generateCsv,
convertStringsToDOMElement,
} from '../componentUtilityFunctions';

const mockData = [
Expand Down Expand Up @@ -128,3 +129,34 @@ describe('componentUtilityFunctions', () => {
expect(splitCsv[4]).toEqual('0');
});
});

describe('convertStringsToDOMElement', () => {
it('accepts, converts, and returns single DOM node wrapped in body', () => {
const [element] = convertStringsToDOMElement(['<div></div>']);
expect(element.tagName).toBe('BODY');
expect(element.querySelectorAll('div').length).toBe(1);
expect(element instanceof Element).toBe(true);
});
it('accepts, converts, and returns multiple DOM nodes', () => {
const [nestedStructure, hasAttribute] = convertStringsToDOMElement([
'<div><ul><li>one</li><li>two</li></ul></div>',
'<a href="ibm.com">IBM</a>',
]);

const listItems = nestedStructure.querySelectorAll('li');
expect(listItems.length).toBe(2);
expect(listItems[0].innerHTML).toBe('one');
expect(listItems[0].parentElement.tagName).toBe('UL');
expect(listItems[0].parentElement.parentElement.tagName).toBe('DIV');

expect(hasAttribute.querySelector('a').hasAttribute('href')).toBe(true);
});
it('returns undefined on empty array', () => {
const [element] = convertStringsToDOMElement([]);
expect(element).toBeUndefined();
});
it('handles text as a parameter', () => {
const [element] = convertStringsToDOMElement(['just a string']);
expect(element.firstChild.textContent).toBe('just a string');
});
});
14 changes: 14 additions & 0 deletions src/utils/componentUtilityFunctions.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -346,3 +346,17 @@ export const browserSupports = (api) => {
export const getOverrides = (props, originalProps) => {
return typeof props === 'function' ? props(originalProps) : props;
};

/**
* Converts strings to DOM Elements, individually returned within a <body> node.
* @param {Array} strings
* @returns {Array} DOM Elements
*/
export const convertStringsToDOMElement = (strings = []) => {
const domparser = new DOMParser();
const mimetype = 'text/html';

return strings.map((string) => {
return domparser.parseFromString(string, mimetype).querySelector('body');
});
};
Loading

0 comments on commit 45f703b

Please sign in to comment.