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

[FieldFormatters] React field formatters #102351

Closed
1 task done
Tracked by #163011
Dosant opened this issue Jun 16, 2021 · 7 comments
Closed
1 task done
Tracked by #163011

[FieldFormatters] React field formatters #102351

Dosant opened this issue Jun 16, 2021 · 7 comments
Labels
Feature:FieldFormatters Icebox impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Meta Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph)

Comments

@Dosant
Copy link
Contributor

Dosant commented Jun 16, 2021

Parent: #65993

Currently, field formatters expose sync API:

const fieldFormatter = getFormatService().deserialize(s.format);
return fieldFormatter.convert(row[s.accessor]);

We should investigate the feasibility of implementing and integrating:

  • Async or stream API.
  • Maybe a field formatter react-renderer wrapper component

We need this to:

Plan:

@Dosant Dosant added Meta loe:needs-research This issue requires some research before it can be worked on or estimated Team:AppServices Feature:FieldFormatters labels Jun 16, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app-services (Team:AppServices)

@Dosant Dosant changed the title [FieldFormatters] Lazy/async/reactive field formatters [FieldFormatters] Async field formatters Jun 16, 2021
@exalate-issue-sync exalate-issue-sync bot added impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort and removed loe:needs-research This issue requires some research before it can be worked on or estimated labels Jun 16, 2021
@exalate-issue-sync exalate-issue-sync bot added loe:medium Medium Level of Effort and removed loe:small Small Level of Effort labels Jul 27, 2021
@Dosant Dosant changed the title [FieldFormatters] Async field formatters [FieldFormatters] Reactive field formatters Oct 11, 2021
@exalate-issue-sync exalate-issue-sync bot added loe:small Small Level of Effort and removed loe:medium Medium Level of Effort labels Nov 17, 2021
@kertal
Copy link
Member

kertal commented Oct 27, 2022

@Dosant wanted to create a own issue for that, but it seems it would be a duplicate, so copying my draft here, would this make sense?

Currenty we have 2 kinds of field formatters, one formatting plain text, and another one for formattinghtml. Therefore, when showing formatted fields in e.g. Discover's Data table, we need to make use of React's dangerouslySetInnerHTML to show the formatted value

<span
className={CELL_CLASS}
// formatFieldValue guarantees sanitized values
// eslint-disable-next-line react/no-danger
dangerouslySetInnerHTML={{
__html: formatFieldValue(row.flattened[columnId], row.raw, fieldFormats, dataView, field),
}}
/>

Having a React based field formatter would give the benefit that no longer dangerouslySetInnerHTML would be necessary.

A field formatter currently has functions for textConvert and htmlConvert, at least one function has to be defined. Additionally there could be another function e.h. reactConvert

export class StringFormat extends FieldFormat {

  textConvert: TextContextTypeConvert = (val: string | number, options) => {
    // code returning plain text formatted value
  };

  htmlConvert: HtmlContextTypeConvert = (val, { hit, field } = {}) => {
   // code returning HTML formatted plain text
 }; 

 reactConvert = (val, { hit, field } = {}) => {
   // returning React element formatting the fields value
 };  
}

Apart from the benefit of having the option to drop the usage of dangerouslySetInnerHTML in the future, this would also give the possibility to create more interactive formatted fields. Like eg. a time fields calculating the time until now, that could automatically refresh this setting.

@Dosant
Copy link
Contributor Author

Dosant commented Oct 31, 2022

@kertal, I think this is would be great. Also easy to fix with it #75729

@Dosant Dosant changed the title [FieldFormatters] Reactive field formatters [FieldFormatters] React field formatters Oct 31, 2022
@petrklapka petrklapka added Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph) and removed Team:AppServicesSv labels Dec 12, 2022
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-data-discovery (Team:DataDiscovery)

@weltenwort weltenwort assigned weltenwort and unassigned weltenwort Apr 11, 2023
@weltenwort
Copy link
Member

@kertal @Dosant, what would your stance be towards replacing the content of this issue's description with the following so that we can pick it up as part of the log explorer work? There is already synchronous imports of React, so this should be independent of the introduction of an async registry. Is there anything you'd like to add to the ACs?

📓 Summary

In order to allow for more dynamic formatting of field values (such as auto-updating relative dates) and full use of interactive EUI components, we want to add a "React" format type in addition to the existing "text" and "html" types.

✔ Acceptance criteria

  • The field format registry supports a new content type react.
  • The field format class supports converting to the new content type react.
  • Discover prefers the react content type, e.g. via its formatFieldValue utility function and doesn't use dangerouslySetInnerHTML if the content type is available.

@kertal
Copy link
Member

kertal commented Apr 25, 2023

Looking good, adding @davismcphee & @mattkime to the loop. Would be great to have this implemented 👍 (and once this is available we should migrate all html formatters to React)

@kertal
Copy link
Member

kertal commented Sep 18, 2024

Closing this because it's not planned to be resolved in the foreseeable future. It will be tracked in our Icebox and will be re-opened if our priorities change. Feel free to re-open if you think it should be melted sooner.

@kertal kertal added the Icebox label Sep 18, 2024
@kertal kertal closed this as not planned Won't fix, can't repro, duplicate, stale Sep 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Feature:FieldFormatters Icebox impact:low Addressing this issue will have a low level of impact on the quality/strength of our product. loe:small Small Level of Effort Meta Team:DataDiscovery Discover App Team (Document Explorer, Saved Search, Surrounding documents, Graph)
Projects
None yet
Development

No branches or pull requests

6 participants