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.

Sign up for GitHub

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

Replace lodash templates with static react renderer for field formatters #96048

Merged
merged 6 commits into from
Apr 3, 2021
Merged

Replace lodash templates with static react renderer for field formatters #96048

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
0eb8790
Replace lodash templates with static react renderer for field formatters
legrego Apr 1, 2021
4cdcf12
remove dangerouslySetInnerHtml
legrego Apr 1, 2021
7cb46f3
Revert "remove dangerouslySetInnerHtml"
legrego Apr 1, 2021
d16d1d4
Attempt 3 for source formatter
legrego Apr 1, 2021
39acb79
closer to original implementation
legrego Apr 1, 2021
773db56
Additional unit test
legrego Apr 2, 2021
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
14 changes: 7 additions & 7 deletions src/plugins/data/common/field_formats/converters/color.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,10 +28,10 @@ describe('Color Format', () => {

expect(colorer.convert(99, HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>99</span>');
expect(colorer.convert(100, HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">100</span></span>'
'<span ng-non-bindable><span style="color:blue;background-color:yellow">100</span></span>'
);
expect(colorer.convert(150, HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">150</span></span>'
'<span ng-non-bindable><span style="color:blue;background-color:yellow">150</span></span>'
);
expect(colorer.convert(151, HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>151</span>');
});
Expand Down Expand Up @@ -74,22 +74,22 @@ describe('Color Format', () => {

expect(converter('B', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>B</span>');
expect(converter('AAA', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AAA</span></span>'
'<span ng-non-bindable><span style="color:blue;background-color:yellow">AAA</span></span>'
);
expect(converter('AB', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB</span></span>'
'<span ng-non-bindable><span style="color:blue;background-color:yellow">AB</span></span>'
);
expect(converter('a', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>a</span>');

expect(converter('B', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>B</span>');
expect(converter('AAA', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AAA</span></span>'
'<span ng-non-bindable><span style="color:blue;background-color:yellow">AAA</span></span>'
);
expect(converter('AB', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB</span></span>'
'<span ng-non-bindable><span style="color:blue;background-color:yellow">AB</span></span>'
);
expect(converter('AB <', HTML_CONTEXT_TYPE)).toBe(
'<span ng-non-bindable><span style="color: blue;background-color: yellow;">AB &lt;</span></span>'
'<span ng-non-bindable><span style="color:blue;background-color:yellow">AB &lt;</span></span>'
);
expect(converter('a', HTML_CONTEXT_TYPE)).toBe('<span ng-non-bindable>a</span>');
});
Expand Down
23 changes: 15 additions & 8 deletions .../common/field_formats/converters/color.ts → ...common/field_formats/converters/color.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@
*/

import { i18n } from '@kbn/i18n';
import { findLast, cloneDeep, template, escape } from 'lodash';
import React from 'react';
import ReactDOM from 'react-dom/server';
import { findLast, cloneDeep, escape } from 'lodash';
import { KBN_FIELD_TYPES } from '../../kbn_field_types/types';
import { FieldFormat } from '../field_format';
import { HtmlContextTypeConvert, FIELD_FORMAT_IDS } from '../types';
import { asPrettyString } from '../utils';
import { DEFAULT_CONVERTER_COLOR } from '../constants/color_default';

const convertTemplate = template('<span style="<%- style %>"><%- val %></span>');

export class ColorFormat extends FieldFormat {
static id = FIELD_FORMAT_IDS.COLOR;
static title = i18n.translate('data.fieldFormats.color.title', {
Expand Down Expand Up @@ -51,11 +51,18 @@ export class ColorFormat extends FieldFormat {

htmlConvert: HtmlContextTypeConvert = (val) => {
const color = this.findColorRuleForVal(val) as typeof DEFAULT_CONVERTER_COLOR;
if (!color) return escape(asPrettyString(val));

let style = '';
if (color.text) style += `color: ${color.text};`;
if (color.background) style += `background-color: ${color.background};`;
return convertTemplate({ val, style });
const displayVal = escape(asPrettyString(val));
if (!color) return displayVal;

return ReactDOM.renderToStaticMarkup(
<span
style={{
color: color.text,
backgroundColor: color.background,
}}
dangerouslySetInnerHTML={{ __html: displayVal }} // eslint-disable-line react/no-danger
/>
);
};
}
52 changes: 24 additions & 28 deletions ...common/field_formats/converters/source.ts → ...ommon/field_formats/converters/source.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,40 +6,34 @@
* Side Public License, v 1.
*/

import { template, escape, keys } from 'lodash';
import React, { Fragment } from 'react';
import ReactDOM from 'react-dom/server';
import { escape, keys } from 'lodash';
import { shortenDottedString } from '../../utils';
import { KBN_FIELD_TYPES } from '../../kbn_field_types/types';
import { FieldFormat } from '../field_format';
import { TextContextTypeConvert, HtmlContextTypeConvert, FIELD_FORMAT_IDS } from '../types';
import { UI_SETTINGS } from '../../constants';

/**
* Remove all of the whitespace between html tags
* so that inline elements don't have extra spaces.
*
* If you have inline elements (span, a, em, etc.) and any
* amount of whitespace around them in your markup, then the
* browser will push them apart. This is ugly in certain
* scenarios and is only fixed by removing the whitespace
* from the html in the first place (or ugly css hacks).
*
* @param {string} html - the html to modify
* @return {string} - modified html
*/
function noWhiteSpace(html: string) {
const TAGS_WITH_WS = />\s+</g;
return html.replace(TAGS_WITH_WS, '><');
interface Props {
defPairs: Array<[string, string]>;
}

const templateHtml = `
<dl class="source truncate-by-height">
<% defPairs.forEach(function (def) { %>
<dt><%- def[0] %>:</dt>
<dd><%= def[1] %></dd>
<%= ' ' %>
<% }); %>
</dl>`;
const doTemplate = template(noWhiteSpace(templateHtml));
const TemplateComponent = ({ defPairs }: Props) => {
return (
<dl className={'source truncate-by-height'}>
{defPairs.map((pair, idx) => (
<Fragment key={idx}>
<dt
dangerouslySetInnerHTML={{ __html: `${pair[0]}:` }} // eslint-disable-line react/no-danger
Copy link
Member Author

Choose a reason for hiding this comment

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

Looking at this one again, I don't think it should be dangerously rendered, unless I call escape. What is your preference / what is the expected behavior?

/>
<dd
dangerouslySetInnerHTML={{ __html: `${pair[1]}` }} // eslint-disable-line react/no-danger
/>{' '}
</Fragment>
))}
</dl>
);
};

export class SourceFormat extends FieldFormat {
static id = FIELD_FORMAT_IDS._SOURCE;
Expand Down Expand Up @@ -70,6 +64,8 @@ export class SourceFormat extends FieldFormat {
pairs.push([newField, val]);
}, []);

return doTemplate({ defPairs: highlightPairs.concat(sourcePairs) });
return ReactDOM.renderToStaticMarkup(
<TemplateComponent defPairs={highlightPairs.concat(sourcePairs)} />
);
};
}