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

fix filter drawing and feature drawing broken in 8.2 #129642

Merged
merged 5 commits into from
Apr 6, 2022

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Apr 6, 2022

Fixes #129632

#113144 added the below redux selector

export const getCustomIcons = ({ map }: MapStoreState): CustomIcon[] => {
  return (
    map.settings.customIcons.map((icon) => {
      return { ...icon, svg: Buffer.from(icon.svg, 'base64').toString('utf-8') };
    }) ?? []
  );
};

This causes problems with react component re-render checking. map.settings.customIcons.map always returns a new array instance so react re-render instance checking will always trigger as updated props.

This exposed an existing bug in draw_control.tsx. Logic in componentDidUpdate set draw control mode to SIMPLE_SELECT when it should not have been. The redux selector bug caused componentDidUpdate to fire on all redux state changes (redux state is updated on mouse move to set the current mouse coordinates).

The fix:

  • draw_control.tsx is updated to better guard the setting of draw control mode to SIMPLE_SELECT
  • getCustomIcons selector returns map.settings.customIcons instead of map.settings.customIcons.map.
  • Base64 encoding/decoding logic is moved to saved_map. Custom icon svg state in redux is always unencoded svg strings. Custom icon svg state in savedObject.attributes is always base64 encoded svg string

@nreese nreese added [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 auto-backport Deprecated - use backport:version if exact versions are needed labels Apr 6, 2022
@nreese nreese requested a review from a team as a code owner April 6, 2022 18:11
@elasticmachine
Copy link
Contributor

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

Copy link
Member

@nickpeihl nickpeihl 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 the fix!

tested in chrome and code review.

@kibana-ci
Copy link
Collaborator

💚 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.5MB 2.5MB +3.7KB
Unknown metric groups

ESLint disabled line counts

id before after diff
maps 42 41 -1

Total ESLint disabled count

id before after diff
maps 70 69 -1

History

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

@nreese nreese merged commit c920640 into elastic:main Apr 6, 2022
kibanamachine pushed a commit that referenced this pull request Apr 6, 2022
* fix filter drawing and freature drawing broken in 8.2

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* tslint

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit c920640)
@kibanamachine
Copy link
Contributor

💚 All backports created successfully

Status Branch Result
8.2

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

kibanamachine added a commit that referenced this pull request Apr 6, 2022
* fix filter drawing and freature drawing broken in 8.2

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

* tslint

* [CI] Auto-commit changed files from 'node scripts/eslint --no-cache --fix'

Co-authored-by: kibanamachine <[email protected]>
(cherry picked from commit c920640)

Co-authored-by: Nathan Reese <[email protected]>
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 [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 v8.2.0 v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] filter drawing and freature drawing broken in 8.2
5 participants