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

[Field Format] Refactor field format to completely remove html format converter #2932

Open
ananzh opened this issue Nov 28, 2022 · 0 comments
Labels
discuss needs research refactor Tech debt related tasks that need refactoring technical debt If not paid, jeapardizes long-term success and maintainability of the repository. unified visualization UX visualizations Issues and PRs related to visualizations

Comments

@ananzh
Copy link
Member

ananzh commented Nov 28, 2022

Field format is restored in index pattern. Currently, we support 15+ formats. An index pattern is constructed like the following

- id: "90943e30-9a47-11e8-b64d-95841ca0b247"
- title: "opensearch_dashboards_sample_data_logs"
- fields (FldList(41)) // contains all field definitions
  for example, for clientip, an IndexPatternField object will be
     displayName: clientip
     spec:
        aggregatable: true
        name: "clientip"
        type: "ip"
        format: DecoratedFieldFormat
              _params
                 parsedUrl
               	  basePath: "/jxv"
                 
                  origin: "http://localhost:5603"
                 
                  pathname: "/jxv/app/home"
            urlTemplate: undefined // it will be reset**
            type: "img" // one format might have diff types

- fieldFormats (FieldFormatsRegistry object which registers about 15 format types)

From @joshuarrrr comment:

It seems the primary purpose of the field formatters (provided by the data plugin) is plain text formatting, which seems both sensible and necessary. I see no need to get rid of these or fundamentally change them.

We have two types of format converters, text and html. The default is text. Only 3 of the formats also have a htmlConvert method:

These return string-ified HTML from templates is wired and requires another layer of transfer. For example, for url formatter there are 3 types. For these 3 types, here is a compare of text converter vs html converter

  • img
"/plugins/indexPatternManagement/assets/icons/177.120.218.48.png"
vs
"<span ng-non-bindable><img src="/plugins/indexPatternManagement/assets/icons/177.120.218.48.png" alt="A dynamically-specified image located at /plugins/indexPatternManagement/assets/icons/177.120.218.48.png" style="width:auto; height:auto; max-width:none; max-height:none;"></span>"
  • audio
"177.120.218.48"
vs
"<span ng-non-bindable><audio controls preload="none" src="177.120.218.48"></span>"
  • link
"177.120.218.48"
vs
"<span ng-non-bindable><a href="http://localhost:5603/jxv/app/177.120.218.48" target="_blank" rel="noopener noreferrer">177.120.218.48</a></span>"

We could see that the text format converter is not able to return correct data. Only html format converter is working for the url format. These return string-ified HTML from templates, so using them in React is awkward and requires dangerouslySetInnerHTML, for example <div dangerouslySetInnerHTML={{ __html: dompurify.sanitize(htmlContent) }} /> , to convert the html content and make it secure.

We should consider refactor field format in the future to completely remove html format converter.

@ananzh ananzh added the visualizations Issues and PRs related to visualizations label Nov 28, 2022
@joshuarrrr joshuarrrr added refactor Tech debt related tasks that need refactoring technical debt If not paid, jeapardizes long-term success and maintainability of the repository. unified visualization UX labels Dec 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss needs research refactor Tech debt related tasks that need refactoring technical debt If not paid, jeapardizes long-term success and maintainability of the repository. unified visualization UX visualizations Issues and PRs related to visualizations
Projects
None yet
Development

No branches or pull requests

3 participants