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

Fix missing html formatting in Doc_Viewer #49326

Merged
merged 5 commits into from
Oct 28, 2019
+20 −197
Merged

Fix missing html formatting in Doc_Viewer #49326

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a68c24f
Add detection if a value has been formatted, conditional rendering
kertal Oct 25, 2019
84619bd
Use markup by formatters, it's escaped for dangerouslySetInnerHTML
kertal Oct 25, 2019
78e98e3
Merge remote-tracking branch 'upstream/master' into kertal-pr-2019-10…
kertal Oct 28, 2019
29ffff2
Enable dangerouslySetInnerHTML for displaying values
kertal Oct 28, 2019
0f86a77
Use regex for replace
Oct 28, 2019
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 src/legacy/core_plugins/kbn_doc_views/public/views/table/table.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ const indexPattern = {
},
metaFields: ['_index', '_score'],
flattenHit: undefined,
formatHit: jest.fn(hit => hit),
formatHit: jest.fn(hit => hit._source),
} as IndexPattern;

indexPattern.flattenHit = flattenHitWrapper(indexPattern, indexPattern.metaFields);
Expand Down
7 changes: 4 additions & 3 deletions src/legacy/core_plugins/kbn_doc_views/public/views/table/table.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
import React, { useState } from 'react';
import { DocViewRenderProps } from 'ui/registry/doc_views';
import { DocViewTableRow } from './table_row';
import { formatValue, arrayContainsObjects } from './table_helper';
import { arrayContainsObjects, trimAngularSpan } from './table_helper';

const COLLAPSE_LINE_LENGTH = 350;

Expand Down Expand Up @@ -48,8 +48,9 @@ export function DocViewTable({
.sort()
.map(field => {
const valueRaw = flattened[field];
const value = formatValue(valueRaw, formatted[field]);
const isCollapsible = typeof value === 'string' && value.length > COLLAPSE_LINE_LENGTH;
const value = trimAngularSpan(String(formatted[field]));

const isCollapsible = value.length > COLLAPSE_LINE_LENGTH;
Copy link
Member Author

Choose a reason for hiding this comment

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

This is the main change, by using the value, formatted by our field formatters, without any component internal modification, it's safe to use dangerouslySetInnerHTML later on. And since this internal formatter isn't used anymore, lot's of helper + test code can be removed.

const isCollapsed = isCollapsible && !fieldRowOpen[field];
const toggleColumn =
onRemoveColumn && onAddColumn && Array.isArray(columns)
Expand Down
133 changes: 1 addition & 132 deletions src/legacy/core_plugins/kbn_doc_views/public/views/table/table_helper.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,91 +16,7 @@
* specific language governing permissions and limitations
* under the License.
*/
import {
replaceMarkWithReactDom,
convertAngularHtml,
arrayContainsObjects,
formatValue,
} from './table_helper';

describe('replaceMarkWithReactDom', () => {
it(`converts <mark>test</mark> to react nodes`, () => {
const actual = replaceMarkWithReactDom(
'<mark>marked1</mark> blablabla <mark>marked2</mark> end'
);
expect(actual).toMatchInlineSnapshot(`
<React.Fragment>
<span>
<mark>
marked1
</mark>
blablabla
</span>
<span>
<mark>
marked2
</mark>
end
</span>
</React.Fragment>
`);
});

it(`doesn't convert invalid markup to react dom nodes`, () => {
const actual = replaceMarkWithReactDom('<mark>test sdf <mark>sdf</mark>');
expect(actual).toMatchInlineSnapshot(`
<React.Fragment>
test sdf
<span>
<mark>
sdf
</mark>
</span>
</React.Fragment>
`);
});

it(`returns strings without markup unchanged `, () => {
const actual = replaceMarkWithReactDom('blablabla');
expect(actual).toMatchInlineSnapshot(`
<React.Fragment>
blablabla
</React.Fragment>
`);
});
});

describe('convertAngularHtml', () => {
it(`converts html for usage in angular to usage in react`, () => {
const actual = convertAngularHtml('<span ng-non-bindable>Good morning!</span>');
expect(actual).toMatchInlineSnapshot(`"Good morning!"`);
});
it(`converts html containing <mark> for usage in react`, () => {
const actual = convertAngularHtml(
'<span ng-non-bindable>Good <mark>morning</mark>dear <mark>reviewer</mark>!</span>'
);
expect(actual).toMatchInlineSnapshot(`
<React.Fragment>
Good
<span>
<mark>
morning
</mark>
dear
</span>
<span>
<mark>
reviewer
</mark>
!
</span>
</React.Fragment>
`);
});
});
import { arrayContainsObjects } from './table_helper';

describe('arrayContainsObjects', () => {
it(`returns false for an array of primitives`, () => {
Expand Down Expand Up @@ -128,50 +44,3 @@ describe('arrayContainsObjects', () => {
expect(actual).toBeFalsy();
});
});

describe('formatValue', () => {
it(`formats an array of objects`, () => {
const actual = formatValue([{ test: '123' }, ''], '');
expect(actual).toMatchInlineSnapshot(`
"{
\\"test\\": \\"123\\"
}
\\"\\""
`);
});
it(`formats an array of primitives`, () => {
const actual = formatValue(['test1', 'test2'], '');
expect(actual).toMatchInlineSnapshot(`"test1, test2"`);
});
it(`formats an object`, () => {
const actual = formatValue({ test: 1 }, '');
expect(actual).toMatchInlineSnapshot(`
"{
\\"test\\": 1
}"
`);
});
it(`formats an angular formatted string `, () => {
const actual = formatValue(
'',
'<span ng-non-bindable>Good <mark>morning</mark>dear <mark>reviewer</mark>!</span>'
);
expect(actual).toMatchInlineSnapshot(`
<React.Fragment>
Good
<span>
<mark>
morning
</mark>
dear
</span>
<span>
<mark>
reviewer
</mark>
!
</span>
</React.Fragment>
`);
});
});
61 changes: 4 additions & 57 deletions src/legacy/core_plugins/kbn_doc_views/public/views/table/table_helper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,70 +16,17 @@
* specific language governing permissions and limitations
* under the License.
*/
import React from 'react';
import { unescape } from 'lodash';

/**
* Convert <mark> markup of the given string to ReactNodes
* @param text
*/
export function replaceMarkWithReactDom(text: string): React.ReactNode {
return (
<>
{text.split('<mark>').map((markedText, idx) => {
const sub = markedText.split('</mark>');
if (sub.length === 1) {
return markedText;
}
return (
<span key={idx}>
<mark>{sub[0]}</mark>
{sub[1]}
</span>
);
})}
</>
);
}

/**
* Current html of the formatter is angular flavored, this current workaround
* should be removed when all consumers of the formatHit function are react based
*/
export function convertAngularHtml(html: string): string | React.ReactNode {
if (typeof html === 'string') {
const cleaned = html.replace('<span ng-non-bindable>', '').replace('</span>', '');
const unescaped = unescape(cleaned);
if (unescaped.indexOf('<mark>') !== -1) {
return replaceMarkWithReactDom(unescaped);
}
return unescaped;
}
return html;
}
/**
* Returns true if the given array contains at least 1 object
*/
export function arrayContainsObjects(value: unknown[]) {
export function arrayContainsObjects(value: unknown[]): boolean {
return Array.isArray(value) && value.some(v => typeof v === 'object' && v !== null);
}

/**
* The current field formatter provides html for angular usage
* This html is cleaned up and prepared for usage in the react world
* Furthermore <mark>test</mark> are converted to ReactNodes
* Removes markup added by kibana fields html formatter
*/
export function formatValue(
value: null | string | number | boolean | object | Array<string | object | null>,
valueFormatted: string
): string | React.ReactNode {
if (Array.isArray(value) && arrayContainsObjects(value)) {
return value.map(v => JSON.stringify(v, null, 2)).join('\n');
} else if (Array.isArray(value)) {
return value.join(', ');
} else if (typeof value === 'object' && value !== null) {
return JSON.stringify(value, null, 2);
} else {
return typeof valueFormatted === 'string' ? convertAngularHtml(valueFormatted) : String(value);
}
export function trimAngularSpan(text: string): string {
return text.replace(/^<span ng-non-bindable>/, '').replace(/<\/span>$/, '');
}
14 changes: 10 additions & 4 deletions src/legacy/core_plugins/kbn_doc_views/public/views/table/table_row.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,17 +85,23 @@ export function DocViewTableRow({
</td>
)}
<td className="kbnDocViewer__field">
<FieldName field={fieldMapping} fieldName={field}></FieldName>
<FieldName field={fieldMapping} fieldName={field} />
</td>
<td>
{isCollapsible && (
<DocViewTableRowBtnCollapse onClick={onToggleCollapse} isCollapsed={isCollapsed} />
)}
{displayUnderscoreWarning && <DocViewTableRowIconUnderscore />}
{displayNoMappingWarning && <DocViewTableRowIconNoMapping />}
<div className={valueClassName} data-test-subj={`tableDocViewRow-${field}-value`}>
{value}
</div>
<div
className={valueClassName}
data-test-subj={`tableDocViewRow-${field}-value`}
/*
* Justification for dangerouslySetInnerHTML:
* We just use values encoded by our field formatters
*/
dangerouslySetInnerHTML={{ __html: value as string }}
/>
</td>
</tr>
);
Expand Down