Skip to content

Conversation

@spalger
Copy link
Contributor

@spalger spalger commented Oct 19, 2016

Fixes #8763

As suggested by @trevan, the default heatMaxZoom of 16 is way too big, and hides complexity in the data that is visible when you set the heatMaxZoom equal to the current zoom level. This pr does just that and removes the option to specify it at all.

I suggest enabling "ignore whitespace" when reviewing the changes.

@epixa
Copy link
Contributor

epixa commented Oct 21, 2016

Since this is breaking, we'll need to make a strong case for it going into a minor.

@epixa
Copy link
Contributor

epixa commented Oct 21, 2016

What happens when a user already has that setting configured on an existing visualization?

@thomasneirynck
Copy link
Contributor

To @epixa's first comment. My preference would be deprecation over removing it as-is.

  • a user could opt-in with a flag in the advanced settings
  • we put the slider in a collapsible panel with a [deprecated] label

Apart from that, this feature may benefit from investigating more why we are recreating the heat-map layer/map itself at each zoom: we really should only recreate those at each change in geohash-precision as they are quite heavy. Of course, we could split that off as a separate performance enhancement/bug fix.

In that light though, by setting the maxZoom to the current zoom level, we are relying a bit on the current inefficiencies for this spot fix to have a good effect. We could also just set maxZoom to '0'. The results for the user are identical and it is more simple as we make fewer assumptions on how this will evolve.

@ppisljar
Copy link
Contributor

ppisljar commented Oct 24, 2016

with current implementation it seems users settings in existing configuration would just be ignored. i agree that if we want to put it in minor version we should do what @thomasneirynck suggests, however that should only be done in backports.

there are few things that are bothering me from user experience perspective:

  • zooming in or out gives weird experience when heatmap is redrawn 2 times
    ibana20161024085154
  • moving around the map causes heatmap to change its intensity colors (for the same spots)
    ibana20161024083413 1
  • also when user zooms in/ out data presented doesn't necesary make sense. for example i am looking at US from a distance and i see really nice heatmap. then i zoom in, suddenly only new york is red. i zoom in more and again there are more red regions ...
    ibana20161024085914

this might be desired (when you want to look at relative values for what's on your screen at the moment) but might not be (if there is low amount of sells in some region i dont want it to be red when i zoom in on it .... )

@tbragin tbragin added the Feature:Visualizations Generic visualization features (in case no more specific feature label is available) label Nov 12, 2016
@spalger spalger added v5.2.0 and removed v5.1.0 labels Nov 21, 2016
@tbragin tbragin removed the v5.2.0 label Jan 4, 2017
@thomasneirynck
Copy link
Contributor

closing this for now. Would prefer to see this resolved together with #10469

@spalger spalger deleted the fix/8763/heatmap-scaling branch October 18, 2019 17:40
acstll added a commit that referenced this pull request Jun 19, 2025
`103.0.0` ⏩ `103.1.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

## Changes

- **EuiDataGrid** now takes an `onFullScreenChange` callback
- An accessibility fix in **EuiComboBox**
- 3 bug fixes

## Package updates

### `@elastic/eui`

## [`v103.1.0`](https://github.com/elastic/eui/releases/v103.1.0)

- Added `onFullScreenChange` prop to `EuiDataGrid` to handle changes
when the component enters or exits fullscreen mode
([#8765](elastic/eui#8765))

**Bug fixes**

- Fixed `onChange` being triggered twice when the checkbox in
`EuiCheckableCard` is clicked
([#8786](elastic/eui#8786))
- Fixed a circular import on the legacy Amsterdam theme that would cause
the theme usage to break
([#8780](elastic/eui#8780))
- Fixed high contrast theme token overrides not being applied
([#8742](elastic/eui#8742))

**Accessibility**

- Fixed form errors not being read by screen readers for `EuiComboBox`
inside of `EuiFormRow`
([#8798](elastic/eui#8798))

<!--ONMERGE {"backportTargets":["8.19","9.0"]} ONMERGE-->
acstll added a commit to acstll/kibana that referenced this pull request Jun 19, 2025
`103.0.0` ⏩ `103.1.0`

[Questions? Please see our Kibana upgrade
FAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)

## Changes

- **EuiDataGrid** now takes an `onFullScreenChange` callback
- An accessibility fix in **EuiComboBox**
- 3 bug fixes

## Package updates

### `@elastic/eui`

## [`v103.1.0`](https://github.com/elastic/eui/releases/v103.1.0)

- Added `onFullScreenChange` prop to `EuiDataGrid` to handle changes
when the component enters or exits fullscreen mode
([elastic#8765](elastic/eui#8765))

**Bug fixes**

- Fixed `onChange` being triggered twice when the checkbox in
`EuiCheckableCard` is clicked
([elastic#8786](elastic/eui#8786))
- Fixed a circular import on the legacy Amsterdam theme that would cause
the theme usage to break
([elastic#8780](elastic/eui#8780))
- Fixed high contrast theme token overrides not being applied
([elastic#8742](elastic/eui#8742))

**Accessibility**

- Fixed form errors not being read by screen readers for `EuiComboBox`
inside of `EuiFormRow`
([elastic#8798](elastic/eui#8798))

<!--ONMERGE {"backportTargets":["8.19","9.0"]} ONMERGE-->

(cherry picked from commit b0d46f7)

# Conflicts:
#	package.json
#	yarn.lock
acstll added a commit that referenced this pull request Jun 19, 2025
# Backport

This will backport the following commits from `main` to `8.19`:
- [Upgrade EUI to v103.1.0
(#224391)](#224391)

<!--- Backport version: 10.0.1 -->

### Questions ?
Please refer to the [Backport tool
documentation](https://github.com/sorenlouv/backport)

<!--BACKPORT [{"author":{"name":"Arturo Castillo
Delgado","email":"[email protected]"},"sourceCommit":{"committedDate":"2025-06-19T08:49:18Z","message":"Upgrade
EUI to v103.1.0 (#224391)\n\n`103.0.0` ⏩ `103.1.0`\n\n[Questions? Please
see our Kibana
upgrade\nFAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)\n\n##
Changes\n\n- **EuiDataGrid** now takes an `onFullScreenChange`
callback\n- An accessibility fix in **EuiComboBox**\n- 3 bug fixes\n\n##
Package updates\n\n### `@elastic/eui`\n\n##
[`v103.1.0`](https://github.com/elastic/eui/releases/v103.1.0)\n\n-
Added `onFullScreenChange` prop to `EuiDataGrid` to handle changes\nwhen
the component enters or exits fullscreen
mode\n([#8765](https://github.com/elastic/eui/pull/8765))\n\n**Bug
fixes**\n\n- Fixed `onChange` being triggered twice when the checkbox
in\n`EuiCheckableCard` is
clicked\n([#8786](https://github.com/elastic/eui/pull/8786))\n- Fixed a
circular import on the legacy Amsterdam theme that would cause\nthe
theme usage to
break\n([#8780](https://github.com/elastic/eui/pull/8780))\n- Fixed high
contrast theme token overrides not being
applied\n([#8742](https://github.com/elastic/eui/pull/8742))\n\n**Accessibility**\n\n-
Fixed form errors not being read by screen readers for
`EuiComboBox`\ninside of
`EuiFormRow`\n([#8798](https://github.com/elastic/eui/pull/8798))\n\n","sha":"b0d46f761d52b65bcd942891f58d68a3bddb13d0","branchLabelMapping":{"^v9.1.0$":"main","^v(\\d+).(\\d+).\\d+$":"$1.$2"}},"sourcePullRequest":{"labels":["review","release_note:skip","EUI","ci:cloud-deploy","backport:version","v9.1.0","v8.19.0"],"title":"Upgrade
EUI to
v103.1.0","number":224391,"url":"https://github.com/elastic/kibana/pull/224391","mergeCommit":{"message":"Upgrade
EUI to v103.1.0 (#224391)\n\n`103.0.0` ⏩ `103.1.0`\n\n[Questions? Please
see our Kibana
upgrade\nFAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)\n\n##
Changes\n\n- **EuiDataGrid** now takes an `onFullScreenChange`
callback\n- An accessibility fix in **EuiComboBox**\n- 3 bug fixes\n\n##
Package updates\n\n### `@elastic/eui`\n\n##
[`v103.1.0`](https://github.com/elastic/eui/releases/v103.1.0)\n\n-
Added `onFullScreenChange` prop to `EuiDataGrid` to handle changes\nwhen
the component enters or exits fullscreen
mode\n([#8765](https://github.com/elastic/eui/pull/8765))\n\n**Bug
fixes**\n\n- Fixed `onChange` being triggered twice when the checkbox
in\n`EuiCheckableCard` is
clicked\n([#8786](https://github.com/elastic/eui/pull/8786))\n- Fixed a
circular import on the legacy Amsterdam theme that would cause\nthe
theme usage to
break\n([#8780](https://github.com/elastic/eui/pull/8780))\n- Fixed high
contrast theme token overrides not being
applied\n([#8742](https://github.com/elastic/eui/pull/8742))\n\n**Accessibility**\n\n-
Fixed form errors not being read by screen readers for
`EuiComboBox`\ninside of
`EuiFormRow`\n([#8798](https://github.com/elastic/eui/pull/8798))\n\n","sha":"b0d46f761d52b65bcd942891f58d68a3bddb13d0"}},"sourceBranch":"main","suggestedTargetBranches":["8.19"],"targetPullRequestStates":[{"branch":"main","label":"v9.1.0","branchLabelMappingKey":"^v9.1.0$","isSourceBranch":true,"state":"MERGED","url":"https://github.com/elastic/kibana/pull/224391","number":224391,"mergeCommit":{"message":"Upgrade
EUI to v103.1.0 (#224391)\n\n`103.0.0` ⏩ `103.1.0`\n\n[Questions? Please
see our Kibana
upgrade\nFAQ.](https://github.com/elastic/eui/blob/main/wiki/eui-team-processes/upgrading-kibana.md#faq-for-kibana-teams)\n\n##
Changes\n\n- **EuiDataGrid** now takes an `onFullScreenChange`
callback\n- An accessibility fix in **EuiComboBox**\n- 3 bug fixes\n\n##
Package updates\n\n### `@elastic/eui`\n\n##
[`v103.1.0`](https://github.com/elastic/eui/releases/v103.1.0)\n\n-
Added `onFullScreenChange` prop to `EuiDataGrid` to handle changes\nwhen
the component enters or exits fullscreen
mode\n([#8765](https://github.com/elastic/eui/pull/8765))\n\n**Bug
fixes**\n\n- Fixed `onChange` being triggered twice when the checkbox
in\n`EuiCheckableCard` is
clicked\n([#8786](https://github.com/elastic/eui/pull/8786))\n- Fixed a
circular import on the legacy Amsterdam theme that would cause\nthe
theme usage to
break\n([#8780](https://github.com/elastic/eui/pull/8780))\n- Fixed high
contrast theme token overrides not being
applied\n([#8742](https://github.com/elastic/eui/pull/8742))\n\n**Accessibility**\n\n-
Fixed form errors not being read by screen readers for
`EuiComboBox`\ninside of
`EuiFormRow`\n([#8798](https://github.com/elastic/eui/pull/8798))\n\n","sha":"b0d46f761d52b65bcd942891f58d68a3bddb13d0"}},{"branch":"8.19","label":"v8.19.0","branchLabelMappingKey":"^v(\\d+).(\\d+).\\d+$","isSourceBranch":false,"state":"NOT_CREATED"}]}]
BACKPORT-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature:Visualizations Generic visualization features (in case no more specific feature label is available) release_note:breaking v6.0.0-rc1 v6.0.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants