Skip to content

[ML] Add Maps ui action to Index data visualizer/Discover's Field statistics#120846

Merged
qn895 merged 6 commits intoelastic:mainfrom
qn895:dv-maps-ui-actions
Dec 14, 2021
Merged

[ML] Add Maps ui action to Index data visualizer/Discover's Field statistics#120846
qn895 merged 6 commits intoelastic:mainfrom
qn895:dv-maps-ui-actions

Conversation

@qn895
Copy link
Copy Markdown
Member

@qn895 qn895 commented Dec 8, 2021

Summary

This PR adds the Maps uiAction to Index data visualizer/Discover's Field statistics:

Screen.Recording.2021-12-08.at.15.40.16.mov
Screen.Recording.2021-12-08.at.15.41.42.mov

@qn895 qn895 self-assigned this Dec 8, 2021
@qn895 qn895 added the Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// label Dec 8, 2021
@qn895 qn895 marked this pull request as ready for review December 9, 2021 05:53
@qn895 qn895 requested a review from a team as a code owner December 9, 2021 05:53
@elasticmachine
Copy link
Copy Markdown
Contributor

Pinging @elastic/ml-ui (:ml)

@elasticmachine
Copy link
Copy Markdown
Contributor

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

Copy link
Copy Markdown
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for adding a way to open geo fields in maps.

Copy link
Copy Markdown
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 latest changes and LGTM

@qn895
Copy link
Copy Markdown
Member Author

qn895 commented Dec 14, 2021

@elasticmachine merge upstream

Copy link
Copy Markdown
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

LGTM, nice and useful addon 👍 , tested and works as expected in Safari, Firefox, Chrome. Just identified a potential cleanup in the code

Comment on lines +394 to +397
JSON.stringify({
searchQueryLanguage,
searchString,
},
actionFlyoutRef
);
if (!Array.isArray(actions) || actions.length < 1) return;

const actionColumn: EuiTableActionsColumnType<FieldVisConfig> = {
name: (
<FormattedMessage
id="xpack.dataVisualizer.index.dataGrid.actionsColumnLabel"
defaultMessage="Actions"
/>
),
actions,
width: '100px',
};

return [actionColumn];
}, [currentIndexPattern, services, searchQueryLanguage, searchString]);

}),
Copy link
Copy Markdown
Member

@kertal kertal Dec 14, 2021

Choose a reason for hiding this comment

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

Just wondering if this is necessary, searchQueryLanguage & searchString are both strings, or am I missing something? so why building an object and then converting it to another string?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for catching that 🙏 . The dependency previously includes a query object. I removed this now here 02db5e1

@qn895 qn895 added auto-backport Deprecated - use backport:version if exact versions are needed and removed auto-backport Deprecated - use backport:version if exact versions are needed labels Dec 14, 2021
@qn895 qn895 enabled auto-merge (squash) December 14, 2021 16:35
@kibana-ci
Copy link
Copy Markdown

💚 Build Succeeded

Metrics [docs]

Async chunks

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

id before after diff
dataVisualizer 535.4KB 536.2KB +847.0B

Page load bundle

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

id before after diff
dataVisualizer 9.7KB 9.8KB +139.0B

History

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

cc @qn895

@qn895 qn895 merged commit 4af8966 into elastic:main Dec 14, 2021
@kibanamachine kibanamachine added the backport:skip This PR does not require backporting label Dec 14, 2021
@qn895 qn895 deleted the dv-maps-ui-actions branch December 21, 2021 18:20
TinLe pushed a commit to TinLe/kibana that referenced this pull request Dec 22, 2021
…tistics (elastic#120846)

* Add map UI actions to dv

* Fix maps logo, comment

* Add permission check for ui actions

* Remove unnecessary json stringify because it no longer includes query object

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

backport:skip This PR does not require backporting Feature:Discover Discover Application Feature:File and Index Data Viz ML file and index data visualizer :ml release_note:enhancement Team:DataDiscovery Discover, search (data plugin and KQL), data views, saved searches. For ES|QL, use Team:ES|QL. t// v8.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants