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

[Maps] fix results trimmed tooltip message doubles feature count for line and polygon features #92932

Merged
merged 2 commits into from
Mar 1, 2021

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Feb 25, 2021

Fixes #92658

This PR removes all uses where featureCollection.features.length is used to determine the number of results. Labeling for lines and polygons adds a centroid feature for each line and polygon to the feature collection so simply looking at features.length could incorrectly make users think double the number of features were returned.

@nreese nreese added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v7.13.0 labels Feb 25, 2021
@nreese nreese requested a review from kindsun February 25, 2021 22:59
@nreese nreese changed the title Issue 92658 [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features Feb 25, 2021
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@@ -75,6 +75,7 @@ export type VectorStyleRequestMeta = MapFilters & {

export type ESSearchSourceResponseMeta = {
areResultsTrimmed?: boolean;
resultsCount?: number;
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be optional? It should always be resp.hits.hits.length so 0 or more, or is that not right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, because meta could be describing search hits or top hits. So resultsCount will not be present when using top hits.

@nreese nreese requested a review from kindsun February 26, 2021 00:35
@nreese
Copy link
Contributor Author

nreese commented Mar 1, 2021

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 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
maps 2.7MB 2.7MB +63.0B
triggersActionsUi 1.6MB 1.5MB -23.9KB
total -23.8KB

Page load bundle

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

id before after diff
triggersActionsUi 104.0KB 104.1KB +82.0B
Unknown metric groups

async chunk count

id before after diff
triggersActionsUi 41 42 +1

History

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

Copy link
Contributor

@kindsun kindsun left a comment

Choose a reason for hiding this comment

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

Overall lgtm! Tested and works as expected

  • code review
  • tested locally in chrome

image

@nreese nreese merged commit 08c4095 into elastic:master Mar 1, 2021
nreese added a commit to nreese/kibana that referenced this pull request Mar 1, 2021
nreese added a commit to nreese/kibana that referenced this pull request Mar 1, 2021
nreese added a commit that referenced this pull request Mar 1, 2021
…line and polygon features (#92932) (#93124)

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
nreese added a commit that referenced this pull request Mar 1, 2021
…line and polygon features (#92932) (#93125)

Co-authored-by: Kibana Machine <[email protected]>

Co-authored-by: Kibana Machine <[email protected]>
gmmorris added a commit to gmmorris/kibana that referenced this pull request Mar 2, 2021
* master: (42 commits)
  [Lens] Introduces new chart switcher (elastic#91844)
  [Lens] fix selection when dragging (elastic#93034)
  Converts usage collection README to .mdx (elastic#92982)
  Fix expanding document when using saved search data grid (elastic#92999)
  [SECURITY SOLUTIONS] Bug case connector (elastic#93104)
  [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (elastic#92025)
  [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (elastic#92942)
  [APM] Fix hidden search bar in error pages while loading (elastic#84476) (elastic#93139)
  [DOCS] Fixes links for machine learning alerts (elastic#92744)
  [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (elastic#92748)
  [SecuritySolution][Case] Disable cases on detections in read-only mode (elastic#93010)
  [Security Solution][Case][Bug] Prevent closing collection when pushing (elastic#93095)
  [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (elastic#92667)
  Bump ems landing page to 7.12 (elastic#93065)
  [App Search] Implement various Relevance Tuning states and form actions (elastic#92644)
  [actions] for simplistic email servers, set rejectUnauthorized to false (elastic#91760)
  [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (elastic#93092)
  Hide instances latency distribution chart (elastic#92869)
  [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (elastic#93087)
  [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (elastic#92932)
  ...
jloleysens added a commit that referenced this pull request Mar 3, 2021
… ilm/rollup-v2-action

* 'ilm/rollup-v2-action' of github.com:elastic/kibana: (30 commits)
  Fix expanding document when using saved search data grid (#92999)
  [SECURITY SOLUTIONS] Bug case connector (#93104)
  [Security Solution] [Timeline] Bugfix to include unmapped fields in the timeline event details JSON (#92025)
  [Alerting][Docs] Changed alerting documentation to point to a single source of explaining the configurations. (#92942)
  [APM] Fix hidden search bar in error pages while loading (#84476) (#93139)
  [DOCS] Fixes links for machine learning alerts (#92744)
  [Security Solution][Detections] -Fixes rule edit flow bug with max_signals (#92748)
  [SecuritySolution][Case] Disable cases on detections in read-only mode (#93010)
  [Security Solution][Case][Bug] Prevent closing collection when pushing (#93095)
  [Security Solution][Detections][7.12] Critical Threshold Rule Fixes (#92667)
  Bump ems landing page to 7.12 (#93065)
  [App Search] Implement various Relevance Tuning states and form actions (#92644)
  [actions] for simplistic email servers, set rejectUnauthorized to false (#91760)
  [Security Solution][Case] Migrate category & subcategory fields of ServiceNow ITSM connector (#93092)
  Hide instances latency distribution chart (#92869)
  [Maps] fix MapboxDraw import from pointing to dist just pointing to folder (#93087)
  [Maps] fix results trimmed tooltip message doubles feature count for line and polygon features (#92932)
  [Security Solution][Detecttions] Indicator enrichment tweaks (#92989)
  [Maps] fix fit to data on heatmap not working (#92697)
  [Security Solution][Endpoint][Admin] Fixes policy sticky footer save test (#92919)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation release_note:skip Skip the PR/issue when compiling release notes v7.12.0 v7.13.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] Results trimmed tooltip message doubles feature count for line and polygon features
4 participants