Fix missing html formatting in Doc_Viewer #49326
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
💔 Build Failed |
💚 Build Succeeded |
…-25-fix-html-fieldformats-in-react
kertal
commented
Oct 28, 2019
| const isCollapsible = typeof value === 'string' && value.length > COLLAPSE_LINE_LENGTH; | ||
| const value = trimAngularSpan(String(formatted[field])); | ||
|
|
||
| const isCollapsible = value.length > COLLAPSE_LINE_LENGTH; |
There was a problem hiding this comment.
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.
💚 Build Succeeded |
kertal
added
the
Team:Visualizations
|
Pinging @elastic/kibana-app (Team:KibanaApp) |
timroes
reviewed
Oct 28, 2019
| return typeof valueFormatted === 'string' ? convertAngularHtml(valueFormatted) : String(value); | ||
| } | ||
| export function trimAngularSpan(text: string): string { | ||
| return text.replace('<span ng-non-bindable>', '').replace('</span>', ''); |
There was a problem hiding this comment.
Just to be save, can we make sure via regex we're stripping here from the beginning and respectively end of the string?
timroes
approved these changes
Oct 28, 2019
💚 Build Succeeded |
timroes
pushed a commit
to timroes/kibana
that referenced
this pull request
Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering * Use markup by formatters, it's escaped for dangerouslySetInnerHTML * Enable dangerouslySetInnerHTML for displaying values * Use regex for replace
timroes
pushed a commit
to timroes/kibana
that referenced
this pull request
Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering * Use markup by formatters, it's escaped for dangerouslySetInnerHTML * Enable dangerouslySetInnerHTML for displaying values * Use regex for replace
timroes
pushed a commit
to timroes/kibana
that referenced
this pull request
Oct 28, 2019
* Add detection if a value has been formatted, conditional rendering * Use markup by formatters, it's escaped for dangerouslySetInnerHTML * Enable dangerouslySetInnerHTML for displaying values * Use regex for replace
timroes
pushed a commit
that referenced
this pull request
Oct 28, 2019
timroes
pushed a commit
that referenced
this pull request
Oct 28, 2019
This was referenced Oct 29, 2019
timroes
pushed a commit
that referenced
this pull request
Oct 29, 2019
16 tasks
kertal
deleted the
kertal-pr-2019-10-25-fix-html-fieldformats-in-react
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
After the refactoring of the DocViewer Table layout to React (#43240), HTML produced by Kibana's Field Formatters was displayed as text, instead of e.g. rendering a Link. This PR restores HTML formatted fields, so e.g. Links provided by the url formatter are clickable again. The HTML provided by the formatters is escaped. This is a interim solution until #48728 will be ready
Checklist
Use
strikethroughsto remove checklist items you don't feel are applicable to this PR.- [ ] This was checked for cross-browser compatibility, including a check against IE11- [ ] Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support- [ ] Documentation was added for features that require explanation or tutorials- [ ] Unit or functional tests were updated or added to match the most common scenarios- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately- [ ] This includes a feature addition or change that requires a release note and was labeled appropriately