-
Notifications
You must be signed in to change notification settings - Fork 8.5k
Toggling spy panel no longer throws an error #8877
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
Conversation
ppisljar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i might be wrong but this seems wrong :) so in dispatch you now always overwrite handler.highlight function with dispatch.highlight .... you defined default noop functions on handler, but those will always get overwriten by dispatch.highligh
src/ui/public/vislib/lib/dispatch.js
Outdated
| if (!this.handler.highlight) { | ||
| this.handler.highlight = self.highlight; | ||
| } | ||
| this.handler.highlight = self.highlight; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so you are always overwriting handler.highlight, even if its already defined. when area chart defines highlight function on handler it will be ignored .... (overwriten by dispatch.highlight)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
great, thanks for flagging this. you're right, this is not going to work since we're also overriding in other places.
1a5f60b to
6fad2be
Compare
|
LGTM, as discussed this is temporary till the vislib rewrite is merged. |
Backports PR #8877 **Commit 1:** Toggling spy panel no longer throws an error * Original sha: 0de553d * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-28T18:06:05Z **Commit 2:** Complete existence check before using handler * Original sha: 6fad2be * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-31T14:11:40Z
Backports PR #8877 **Commit 1:** Toggling spy panel no longer throws an error * Original sha: 0de553d * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-28T18:06:05Z **Commit 2:** Complete existence check before using handler * Original sha: 6fad2be * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-31T14:11:40Z
Backports PR #8877 **Commit 1:** Toggling spy panel no longer throws an error * Original sha: 0de553d * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-28T18:06:05Z **Commit 2:** Complete existence check before using handler * Original sha: 6fad2be * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-31T14:11:40Z
Complete existence check of highlight-handler, before using it
Backports PR elastic#8877 **Commit 1:** Toggling spy panel no longer throws an error * Original sha: 0de553d * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-28T18:06:05Z **Commit 2:** Complete existence check before using handler * Original sha: 6fad2be * Authored by Thomas Neirynck <thomas@elastic.co> on 2016-10-31T14:11:40Z Former-commit-id: c834ee7
`@elastic/eui`: `106.0.0` ⏩ `106.1.0` `@elastic/eslint-plugin-eui`: `2.1.0` ⏩ `2.2.1` [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) New props and defaults have been added to `EuiFlyout`, including `includeSelectorInFocusTrap`, plus new global CSS variables for push mode offset control. Additionally, we introduced a small spacing fix for invalid `EuiDualRange` append component and added support for custom styles on `EuiTreeView` nodes via the css key. :warning: **React 16 is now deprecated:** EUI no longer tests against it or fixes issues specific to it. ## Changes - I removed forced click in: `x-pack/test/security_solution_cypress/cypress/tasks/rule_details.ts` - I removed `{enter}` in: `x-pack/test/security_solution_cypress/cypress/tasks/common/rule_actions.ts` (see #228517) ## Package updates ### `@elastic/eui` #### [`v106.1.0`](https://github.com/elastic/eui/releases/v106.1.0) - Added `--euiPushFlyoutOffsetInlineStart` and `--euiPushFlyoutOffsetInlineEnd` global CSS variables set by the `EuiFlyout` in `push` mode. ([#8872](elastic/eui#8872)) - Reduced the `min-width` for inputs in `EuiRange` and `EuiDualRange` ([#8866](elastic/eui#8866)) - Added `includeSelectorInFocusTrap` prop for `EuiFlyout` ([#8849](elastic/eui#8849)) - Added component defaults for `EuiFlyout` that include `includeSelectorInFocusTrap` and `includeFixedHeadersInFocusTrap` ([#8849](elastic/eui#8849)) **Bug fixes** - Fixed flaky manual return focus behavior on `EuiFlyout` by relying on `FocusTrap` returning focus instead ([#8878](elastic/eui#8878)) - Fixed `EuiFlyoutChild` closing its parent `EuiFlyout` on `Escape` keypress ([#8878](elastic/eui#8878)) - Fixed support for `css` key in items object passed to `EuiTreeView` ([#8864](elastic/eui#8864)) **Deprecations** - Deprecated support for React 16. Please update to a modern version of React or stay on EUI version 106.0.0 if you can't switch right now. ([#8868](elastic/eui#8868)) ### `@elastic/eslint-plugin-eui` #### [`v2.2.1`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.1) **Bug fixes** - Removed `no-css_color` entry which mapped to a duplicate `no-css-color` rule ([#8888](elastic/eui#8888)) #### [`v2.2.0`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.0) - Added new `prefer-eui-icon-tip` rule. ([#8877](elastic/eui#8877))
`@elastic/eui`: `106.0.0` ⏩ `106.1.0` `@elastic/eslint-plugin-eui`: `2.1.0` ⏩ `2.2.1` [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) New props and defaults have been added to `EuiFlyout`, including `includeSelectorInFocusTrap`, plus new global CSS variables for push mode offset control. Additionally, we introduced a small spacing fix for invalid `EuiDualRange` append component and added support for custom styles on `EuiTreeView` nodes via the css key. :warning: **React 16 is now deprecated:** EUI no longer tests against it or fixes issues specific to it. ## Changes - I removed forced click in: `x-pack/test/security_solution_cypress/cypress/tasks/rule_details.ts` - I removed `{enter}` in: `x-pack/test/security_solution_cypress/cypress/tasks/common/rule_actions.ts` (see elastic#228517) ## Package updates ### `@elastic/eui` #### [`v106.1.0`](https://github.com/elastic/eui/releases/v106.1.0) - Added `--euiPushFlyoutOffsetInlineStart` and `--euiPushFlyoutOffsetInlineEnd` global CSS variables set by the `EuiFlyout` in `push` mode. ([elastic#8872](elastic/eui#8872)) - Reduced the `min-width` for inputs in `EuiRange` and `EuiDualRange` ([elastic#8866](elastic/eui#8866)) - Added `includeSelectorInFocusTrap` prop for `EuiFlyout` ([elastic#8849](elastic/eui#8849)) - Added component defaults for `EuiFlyout` that include `includeSelectorInFocusTrap` and `includeFixedHeadersInFocusTrap` ([elastic#8849](elastic/eui#8849)) **Bug fixes** - Fixed flaky manual return focus behavior on `EuiFlyout` by relying on `FocusTrap` returning focus instead ([elastic#8878](elastic/eui#8878)) - Fixed `EuiFlyoutChild` closing its parent `EuiFlyout` on `Escape` keypress ([elastic#8878](elastic/eui#8878)) - Fixed support for `css` key in items object passed to `EuiTreeView` ([elastic#8864](elastic/eui#8864)) **Deprecations** - Deprecated support for React 16. Please update to a modern version of React or stay on EUI version 106.0.0 if you can't switch right now. ([elastic#8868](elastic/eui#8868)) ### `@elastic/eslint-plugin-eui` #### [`v2.2.1`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.1) **Bug fixes** - Removed `no-css_color` entry which mapped to a duplicate `no-css-color` rule ([elastic#8888](elastic/eui#8888)) #### [`v2.2.0`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.0) - Added new `prefer-eui-icon-tip` rule. ([elastic#8877](elastic/eui#8877))
`@elastic/eui`: `106.0.0` ⏩ `106.1.0` `@elastic/eslint-plugin-eui`: `2.1.0` ⏩ `2.2.1` [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) New props and defaults have been added to `EuiFlyout`, including `includeSelectorInFocusTrap`, plus new global CSS variables for push mode offset control. Additionally, we introduced a small spacing fix for invalid `EuiDualRange` append component and added support for custom styles on `EuiTreeView` nodes via the css key. :warning: **React 16 is now deprecated:** EUI no longer tests against it or fixes issues specific to it. ## Changes - I removed forced click in: `x-pack/test/security_solution_cypress/cypress/tasks/rule_details.ts` - I removed `{enter}` in: `x-pack/test/security_solution_cypress/cypress/tasks/common/rule_actions.ts` (see elastic#228517) ## Package updates ### `@elastic/eui` #### [`v106.1.0`](https://github.com/elastic/eui/releases/v106.1.0) - Added `--euiPushFlyoutOffsetInlineStart` and `--euiPushFlyoutOffsetInlineEnd` global CSS variables set by the `EuiFlyout` in `push` mode. ([elastic#8872](elastic/eui#8872)) - Reduced the `min-width` for inputs in `EuiRange` and `EuiDualRange` ([elastic#8866](elastic/eui#8866)) - Added `includeSelectorInFocusTrap` prop for `EuiFlyout` ([elastic#8849](elastic/eui#8849)) - Added component defaults for `EuiFlyout` that include `includeSelectorInFocusTrap` and `includeFixedHeadersInFocusTrap` ([elastic#8849](elastic/eui#8849)) **Bug fixes** - Fixed flaky manual return focus behavior on `EuiFlyout` by relying on `FocusTrap` returning focus instead ([elastic#8878](elastic/eui#8878)) - Fixed `EuiFlyoutChild` closing its parent `EuiFlyout` on `Escape` keypress ([elastic#8878](elastic/eui#8878)) - Fixed support for `css` key in items object passed to `EuiTreeView` ([elastic#8864](elastic/eui#8864)) **Deprecations** - Deprecated support for React 16. Please update to a modern version of React or stay on EUI version 106.0.0 if you can't switch right now. ([elastic#8868](elastic/eui#8868)) ### `@elastic/eslint-plugin-eui` #### [`v2.2.1`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.1) **Bug fixes** - Removed `no-css_color` entry which mapped to a duplicate `no-css-color` rule ([elastic#8888](elastic/eui#8888)) #### [`v2.2.0`](https://www.npmjs.com/package/@elastic/eslint-plugin-eui/v/2.2.0) - Added new `prefer-eui-icon-tip` rule. ([elastic#8877](elastic/eui#8877))
Closes #8869.
Also tidied up some linter warnings.