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

[ML] Data visualizer: Add field types in-product help #137121

Merged
merged 5 commits into from
Jul 26, 2022

Conversation

qn895
Copy link
Member

@qn895 qn895 commented Jul 25, 2022

Summary

This PR addresses #131577 and is part of #86387. It adds the popover with the help information for each field type.

Index based

Screen Shot 2022-07-26 at 12 48 41

File based

Screen Shot 2022-07-26 at 12 49 26

Checklist

Fixes #131577

@qn895 qn895 force-pushed the ml-dv-field-type-help-pop-over branch from 48b5c7b to 69cd035 Compare July 25, 2022 21:27
@qn895 qn895 added the Feature:File and Index Data Viz ML file and index data visualizer label Jul 25, 2022
@qn895 qn895 marked this pull request as ready for review July 25, 2022 21:34
@qn895 qn895 requested a review from a team as a code owner July 25, 2022 21:34
@elasticmachine
Copy link
Contributor

Pinging @elastic/ml-ui (:ml)

defaultMessage: 'Software versions. Supports {SemanticVersioningLink} precedence rules.',
values: {
SemanticVersioningLink:
`<a href="https://semver.org/"
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason this has to be a string of an a tag as opposed to a link component from the eui library?

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 originally copied from Discover. Since this is in a constant file used in a helper function, using EuiLink would require this file to be importing React. However, if readability is important, we can definitely switch over.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sounds good 👍

const closeHelp = () => setIsHelpOpen(false);

const items: FieldTypeTableItem[] = useMemo(() => {
return fieldTypes.map((type, index) => ({
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: could be implicit return

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

defaultMessage: 'Description',
}),
// eslint-disable-next-line react/no-danger
render: (description: string) => <div dangerouslySetInnerHTML={{ __html: description }} />,
Copy link
Contributor

Choose a reason for hiding this comment

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

The description here is only ever set internally, right? Just making sure we want use the set inner html

Copy link
Member Author

Choose a reason for hiding this comment

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

I realized we don't even need it so I removed that part here 024eac1

Copy link
Contributor

@lcawl lcawl left a comment

Choose a reason for hiding this comment

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

Minor comments about the punctuation, otherwise text strings LGTM

switch (type) {
case SUPPORTED_FIELD_TYPES.BOOLEAN:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.booleanField', {
defaultMessage: 'True and false values.',
Copy link
Contributor

Choose a reason for hiding this comment

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

These incomplete phrases don't warrant punctuation IMO

Suggested change
defaultMessage: 'True and false values.',
defaultMessage: 'True and false values',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.DATE:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.dateField', {
defaultMessage: 'A date string or the number of seconds or milliseconds since 1/1/1970.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'A date string or the number of seconds or milliseconds since 1/1/1970.',
defaultMessage: 'A date string or the number of seconds or milliseconds since 1/1/1970',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.GEO_POINT:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.geoPointField', {
defaultMessage: 'Latitude and longitude points.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Latitude and longitude points.',
defaultMessage: 'Latitude and longitude points',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.GEO_SHAPE:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.geoShapeField', {
defaultMessage: 'Complex shapes, such as polygons.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Complex shapes, such as polygons.',
defaultMessage: 'Complex shapes such as polygons',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.HISTOGRAM:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.histogramField', {
defaultMessage: 'Pre-aggregated numerical values in the form of a histogram.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Pre-aggregated numerical values in the form of a histogram.',
defaultMessage: 'Pre-aggregated numerical values in the form of a histogram',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.NUMBER:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.numberField', {
defaultMessage: 'Long, integer, short, byte, double, and float values.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Long, integer, short, byte, double, and float values.',
defaultMessage: 'Long, integer, short, byte, double, and float values',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.STRING:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.stringField', {
defaultMessage: 'Full text such as the body of an email or a product description.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Full text such as the body of an email or a product description.',
defaultMessage: 'Full text such as the body of an email or a product description',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.TEXT:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.textField', {
defaultMessage: 'Full text such as the body of an email or a product description.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Full text such as the body of an email or a product description.',
defaultMessage: 'Full text such as the body of an email or a product description',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

case SUPPORTED_FIELD_TYPES.KEYWORD:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.keywordField', {
defaultMessage:
'Structured content such as an ID, email address, hostname, status code, or tag.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
'Structured content such as an ID, email address, hostname, status code, or tag.',
'Structured content such as an ID, email address, hostname, status code, or tag',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

});
case SUPPORTED_FIELD_TYPES.VERSION:
return i18n.translate('xpack.dataVisualizer.index.fieldNameDescription.versionField', {
defaultMessage: 'Software versions. Supports {SemanticVersioningLink} precedence rules.',
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
defaultMessage: 'Software versions. Supports {SemanticVersioningLink} precedence rules.',
defaultMessage: 'Software versions. Supports {SemanticVersioningLink} precedence rules',

Copy link
Member Author

Choose a reason for hiding this comment

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

Updated here 024eac1

Copy link
Contributor

@peteharverson peteharverson left a comment

Choose a reason for hiding this comment

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

Tested and other than the changes already suggested LGTM

Copy link
Contributor

@alvarezmelissa87 alvarezmelissa87 left a comment

Choose a reason for hiding this comment

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

LGTM ⚡

@kibana-ci
Copy link
Collaborator

💚 Build Succeeded

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id before after diff
dataVisualizer 377 378 +1

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
dataVisualizer 561.6KB 565.1KB +3.5KB

Page load bundle

Size of the bundles that are downloaded on every page load. Target size is below 100kb

id before after diff
dataVisualizer 16.6KB 20.4KB +3.8KB
Unknown metric groups

ESLint disabled line counts

id before after diff
dataVisualizer 38 39 +1

Total ESLint disabled count

id before after diff
dataVisualizer 38 39 +1

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

cc @qn895

@qn895 qn895 merged commit 0cf9cb2 into elastic:main Jul 26, 2022
@kibanamachine kibanamachine added the backport:skip This commit does not require backporting label Jul 26, 2022
@qn895 qn895 deleted the ml-dv-field-type-help-pop-over branch August 10, 2022 16:38
@tylersmalley tylersmalley added ci:cloud-deploy Create or update a Cloud deployment and removed ci:deploy-cloud labels Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport:skip This commit does not require backporting ci:cloud-deploy Create or update a Cloud deployment Feature:File and Index Data Viz ML file and index data visualizer :ml release_note:enhancement v8.4.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[ML] Add field types in-product help
8 participants