Skip to content

[XY axis] Fixes filters not work for charts with percentage mode.#100456

Merged
Kuznietsov merged 2 commits intoelastic:masterfrom
Kuznietsov:#99906
May 26, 2021
Merged

[XY axis] Fixes filters not work for charts with percentage mode.#100456
Kuznietsov merged 2 commits intoelastic:masterfrom
Kuznietsov:#99906

Conversation

@Kuznietsov
Copy link
Contributor

@Kuznietsov Kuznietsov commented May 24, 2021

Fixes #99906

Before this part of the code was comparing clean data, which came from the dataset, with X/Y values. They were true according to normal mode, but in percentage mode, for example, it was comparing the absolute value with a percentage value. To avoid it, need to compare datum (feature elastic/elastic-charts#822) of geometry with the clean value from row info.

Before this part of code was comparing clean data, which came from dataset, with X/Y values. They were true according to normal mode, but in percentage mode, for example, it was comparing absolute value with percentage value. To avoid it, need to compare datum (feature elastic#822 from elastic/elastic-charts) of geometry with clean value from row info.
@kibanamachine
Copy link
Contributor

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@alexwizp alexwizp changed the title Fixes [XY axis] Filters not work for charts with percentage mode. [XY axis] Fixes filters not work for charts with percentage mode. May 24, 2021
@alexwizp alexwizp requested a review from nickofthyme May 25, 2021 13:34
@alexwizp alexwizp added Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.14.0 v8.0.0 Feature:XYAxis XY-Axis charts (bar, area, line) release_note:fix auto-backport Deprecated - use backport:version if exact versions are needed labels May 25, 2021
@alexwizp
Copy link
Contributor

alexwizp commented May 25, 2021

@nickofthyme could you please have a look? I've tested it locally and that changes fix reported problem.

Copy link
Contributor

@nickofthyme nickofthyme left a comment

Choose a reason for hiding this comment

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

LGTM. Good catch, I didn't even think about the percentage value being the returned value.

@alexwizp alexwizp marked this pull request as ready for review May 25, 2021 19:20
@alexwizp alexwizp requested a review from a team May 25, 2021 19:20
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-app (Team:KibanaApp)

@alexwizp
Copy link
Contributor

@elasticmachine merge upstream

Copy link
Contributor

@stratoula stratoula left a comment

Choose a reason for hiding this comment

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

This works great, thank you @kuznetsov-y, changes LGTM

@stratoula
Copy link
Contributor

Jenkins, test this

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Page load bundle

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

id before after diff
charts 86.5KB 86.6KB +64.0B
Unknown metric groups

References to deprecated APIs

id before after diff
canvas 29 25 -4
crossClusterReplication 8 6 -2
fleet 22 20 -2
globalSearch 4 2 -2
indexManagement 12 7 -5
infra 261 149 -112
lens 67 45 -22
licensing 18 15 -3
lists 239 236 -3
maps 286 208 -78
ml 121 115 -6
monitoring 109 56 -53
securitySolution 386 342 -44
stackAlerts 101 95 -6
total -342

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

cc @kuznetsov-y

@kibanamachine
Copy link
Contributor

💚 Backport successful

Status Branch Result
7.x

This backport PR will be merged automatically after passing CI.

kibanamachine added a commit to kibanamachine/kibana that referenced this pull request May 26, 2021
…#100456)

Before this part of code was comparing clean data, which came from dataset, with X/Y values. They were true according to normal mode, but in percentage mode, for example, it was comparing absolute value with percentage value. To avoid it, need to compare datum (feature elastic#822 from elastic/elastic-charts) of geometry with clean value from row info.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
@Kuznietsov Kuznietsov deleted the #99906 branch May 26, 2021 08:42
jloleysens added a commit to jloleysens/kibana that referenced this pull request May 26, 2021
…deprecation-ilm-policy

* 'master' of github.com:elastic/kibana: (101 commits)
  [ftr] migrate "docTable" service to FtrService class (elastic#100595)
  [ftr] migrate "listingTable" service to FtrService class (elastic#100606)
  Fixed comparing real value with formatted according to mode. (elastic#100456)
  [ftr] migrate "dataGrid" service to FtrService class (elastic#100593)
  [ftr] migrate "fieldEditor" to FtrService class (elastic#100597)
  [ftr] migrate "filterBar" service to FtrService class (elastic#100601)
  [triggersActionsUi] Reduce page load bundle to under 100kB (elastic#97770)
  [build] Clean jest configs (elastic#100594)
  refact(NA): remove extra pkg_npm target and add specific target folders for @kbn/analytics on Bazel (elastic#100569)
  Update dependency @elastic/charts to v29.2.0 (elastic#100587)
  [Maps] convert LayerPanel to typescript (elastic#100481)
  [Upgrade Assistant] Address copy feedback (elastic#99632)
  Open/Closed filter for observability alerts page (elastic#99217)
  One liner to expose the EQL query for debugging for users (elastic#100565)
  [KibanaPageLayout] Solution Nav specific styles & props (elastic#100089)
  [ftr] implement FtrService classes and migrate common services (elastic#99546)
  [XY] [Lens] Adds opacity slider (elastic#100453)
  [Reporting] ILM policy for managing reporting indices (elastic#100130)
  [Reporting] ILM policy for managing reporting indices (elastic#100130)
  [DOCS] Remove redundant maps attribute (elastic#100426)
  ...

# Conflicts:
#	x-pack/plugins/reporting/server/lib/store/report_ilm_policy.ts
#	x-pack/plugins/reporting/server/lib/store/store.test.ts
#	x-pack/plugins/reporting/server/lib/store/store.ts
kibanamachine added a commit that referenced this pull request May 26, 2021
#100636)

Before this part of code was comparing clean data, which came from dataset, with X/Y values. They were true according to normal mode, but in percentage mode, for example, it was comparing absolute value with percentage value. To avoid it, need to compare datum (feature #822 from elastic/elastic-charts) of geometry with clean value from row info.

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>

Co-authored-by: Yaroslav Kuznietsov <kuznetsov.yaroslav.yk@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

auto-backport Deprecated - use backport:version if exact versions are needed Feature:XYAxis XY-Axis charts (bar, area, line) release_note:fix Team:Visualizations Team label for Lens, elastic-charts, Graph, legacy editors (TSVB, Visualize, Timelion) t// v7.14.0 v8.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[XY axis] Filters not work for charts with percentage mode

7 participants