Bumping EUI to 71.0.0 #147142

breehall · 2022-12-06T21:02:01Z

`71.0.0`

Implemented new EuiRange and EuiDualRange designs where the levels are now on top of the tracks (#6092)
Added discuss and dotInCircle glyphs to EuiIcon (#6434)
Added article glyph to EuiIcon (#6437)
Changed the EuiProvider usage warnings to not rely on development mode. (#6451)

Breaking changes

EuiDualRange now explicitly requires both min and max via props types, to match EuiRange (#6092)
EuiRange and EuiDualRange's compressed size no longer impacts track or level sizes, but continues to compress tick and input sizes. (#6092)
Removed all variables for the following components from EUI's theme JSON files: (#6443)
- euiCollapsibleNav*
- euiColorPicker*
- euiContextMenu*
- euiControlBar*
- euiDataGrid* (except for z-indices and cell padding sizes)
- euiDatePicker*
- euiSuperDatePicker*
- euiDragAndDrop*
- euiEuiEmptyPrompt*
- euiFilePicker*
- euiRange*
- euiHeaderLinks*
- euiKeyPad*
- euiMarkdownEditor*
- euiResizable*
- euiSelectable*
- euiSideNav*
- euiStep*
- euiSuggest*
- euiTable* (except for color variables)
- euiTooltip*
- euiButtonFontWeight, euiButtonDefaultTransparency, and euiButtonMinWidth
If you were importing any of the above removed JSON variables, we strongly recommend using generic color or sizing variables from useEuiTheme() instead. (#6443)

CSS-in-JS conversions

Converted EuiRange and EuiDualRange to Emotion; Removed $euiRangeThumbRadius (#6092)
Added a new logicalStyles utility that automatically converts all non-logical properties in a style object to their corresponding logical properties (#6426)
Added a new logicalShorthandCSS utility that automatically converts margin, padding, and other 4-sided shorthands to their corresponding logical properties (#6429)
Added a new logicalBorderRadiusCSS utility that automatically converts border-radius to corresponding logical properties (#6429)

- the styling is significantly different on EuiRangeLevels and any previous tweaks likely should not carry over - also these classes no longer exist and as such their selectors no longer apply

- now that the component is wrapped in `withEuiTheme`, unfortunately things are extremely messy - unfortunately converting EuiDualRange to a function component is not necesssarily the answer as these specific use cases then lose access to the `onResize` instance method + fix incorrect usage of `rangeRef` in `src/plugins/controls` - the rangeRef was never actually getting passed down and thus never actually calling `onResize` - also `onResize` needs an actual width number

… Emotion

… attribute was added in PR #6445

…e in the EuiText class name that required a snapshot update

…e in the EuiText class name that required a snapshots update. [ScatterPlots] Updated the grayboilder plate and ticks/grid/axis colors within the ML scatterplt matrix wizard to match the EUI darkest shade

…the range slider value

cee-chen · 2022-12-14T00:18:57Z

@elasticmachine merge upstream

…Wizard

…ui-71.0.0 Merging in latest code from EUI upgrade branch

breehall · 2022-12-14T16:19:18Z

...analytics/pages/analytics_creation/components/configuration_step/configuration_step_form.tsx

-        ...(depVarIsRuntimeField || jobTypeChanged || depVarNotIncluded
-          ? { includes: formToUse.includes }
-          : {}),


jobTypeChanged was removed here because it caused extra renders of the component that prevented the includedFields table on the Regression job page not to be automatically populated when dependentVariable was selected. This occurred because this condition was detecting a job change when there wasn't one.

elasticmachine · 2022-12-14T16:20:04Z

Pinging @elastic/eui-design (EUI)

elasticmachine · 2022-12-14T16:33:16Z

Pinging @elastic/uptime (Team:uptime)

patrykkopycinski

Asset management LGTM

thomheymann

security changes LGTM!

alisonelizabeth

Deployment Management changes LGTM

nreese · 2022-12-14T18:24:53Z

...s/maps/public/classes/styles/vector/components/data_mapping/ordinal_data_mapping_popover.tsx

@@ -18,6 +18,7 @@ import {
  EuiText,
  EuiToolTip,
 } from '@elastic/eui';
+import type { _SingleRangeChangeEvent } from '@elastic/eui/src/components/form/range/types';


Instead of _SingleRangeChangeEvent, would it be possible to use EuiRangeProps['onChange'], like in https://github.com/elastic/kibana/blob/56e45b19c57371599ca0e164bfec1b417f1aa91a/x-pack/plugins/maps/public/classes/sources/es_geo_grid_source/resolution_editor.tsx?

Unfortunately not because of the syntax used. onSigmaChange is using function onSigmaChange() {} as opposed to const onSigmaChange = () => {}, and cannot be typed to a function type (EuiRangeProps['onChange']) the same way as consts can (at least, I couldn't find a way to accomplish this in my google journeys - I'd love to be wrong on this, please tell me if you know of a way to do so!)

I also attempted to use Parameters<EuiRangeProps['onChange']>[0] but unfortunately Typescript couldn't figure that out - I think possibly due to some bizarre combination of reaching into an interface for a key and that key being optional/potentially undefined.

Importing the event type directly was the easiest way I could figure out to update typings without changing your source code, but if you're okay with changing your syntax - sure, we can make that change :)

ThomThomson

Reviewed this locally, paying special attention to the sliders, and everything looks good! The new design works well. Left a few questions about some of the changes.

ThomThomson · 2022-12-14T20:27:35Z

src/plugins/controls/public/range_slider/components/range_slider_popover.tsx

-            min={hasAvailableRange ? rangeSliderMin : undefined}
-            max={hasAvailableRange ? rangeSliderMax : undefined}
+            min={hasAvailableRange ? rangeSliderMin : 0}
+            max={hasAvailableRange ? rangeSliderMax : 100}


Tested this by getting the range slider control into a no-data state, and it looks the same as it did before. 👌

ThomThomson · 2022-12-14T20:28:19Z

src/plugins/controls/public/range_slider/components/range_slider_popover.tsx

@@ -26,9 +27,11 @@ import { rangeSliderReducers } from '../range_slider_reducers';
 import { RangeSliderReduxState } from '../types';
 import { RangeSliderStrings } from './range_slider_strings';

-export const RangeSliderPopover: FC = () => {
+// Unfortunately, wrapping EuiDualRange in `withEuiTheme` has created this annoying/verbose typing


Is there anything we can do on our side to get rid of these complex types? Maybe a forward ref? Or some other way of getting a handle on the class?

Honestly? Without knowing exactly why you're calling the internal EuiDualRange's internal onResize class method... my recommendation is to not do that 😅 (which removes the need for the ref entirely, and thus the ref typing shenanigans).

If we ever convert EuiRange/EuiDualRange to functional non-class components as a tech debt item, you'd lose access to this API entirely (it's not something we're deliberately exposing or documenting as a consumer API). I did create a regression test to ensure we continued exposing onResize via ref, just for this specific Kibana usage of it, but honestly, we'd prefer y'all not reach for it at all, or figure out what problem it's solving and have us try to solve it in a less technically hacky way.

I don't have the context as to why that original call was added or what it's doing - but it's definitely a code smell that makes me think either this usage of it is trying to do things EuiDualRange is not meant to do, or is super custom and undoing things EuiDualRange does and then re-doing them later (also not great).

If I remember correctly, this usage is due to us not being able to use EuiInputPopover, but still wanting to have the popover to be tied to the width of the input. I can't exactly remember why we couldn't use EuiInputPopover though.

I wonder if we would run into any regressions by removing that call entirely? @cqliu1, do you remember why we had to use EuiPopover?

ThomThomson · 2022-12-14T20:29:52Z

x-pack/plugins/canvas/canvas_plugin_src/uis/arguments/partition_labels/extended_template.tsx

-        event: ChangeEvent<HTMLInputElement | HTMLSelectElement> | MouseEvent<HTMLButtonElement>
+        event:
+          | ChangeEvent<HTMLInputElement | HTMLSelectElement>
+          | KeyboardEvent<HTMLInputElement>


What exactly changed here requiring the addition of the KeyboardEvent type?

A slightly younger and far more naive version of me was looking at the EuiRange types and was like, "keyboard users / keyboard events on the input are also a thing! Let me modify our event types to add that, surely there won't be that many Kibana usages for us to update, right?"

Narrator: There were that many Kibana usages.

So yeah, that change is squarely my fault. The type change didn't really have to happen and was just me going "hm this is probably nice to have" and not realizing it would affect so many downstream Kibana consumers 😭 On the plus side, 6f1dfd2 dried out statically copied types for multiple instances/usages, so for most cases (except this one, where I couldn't extend EuiRangeProps['onChange'] for whatever arcane Typescript reason - see an earlier comment about Parameters<>), this was generally a very small net cleanup.

stratoula

Visualizations changes LGTM, I did a brief testing in Lens and agg based visualizations that use ranges and everything works as expected.

sphilipse

Enterprise Search changes LGTM

spong

Checked out, tested locally, and both the AnomalyThresholdSlider and the RiskScoreMapping EuiRange components are functioning as expected within Security Solution. Security Detections changes LGTM! 👍 Thanks @breehall! 🙂

michaelolo24

Security investigations review! LGTM!

peteharverson

ML changes LGTM

cee-chen · 2022-12-19T17:28:09Z

@elasticmachine merge upstream

opauloh

awp-viz lgtm

- use a more generic version instead

kibana-ci · 2022-12-19T19:29:07Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: 985f451

Failed CI Steps

Osquery Cypress Tests

Metrics [docs]

Module Count

Fewer modules leads to a faster build time

id	before	after	diff
`infra`	1216	1217	+1
`visTypeVega`	307	308	+1
total			+2

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id	before	after	diff
`controls`	454.3KB	454.3KB	-1.0B
`enterpriseSearch`	2.0MB	2.0MB	+7.0B
`infra`	1.2MB	1.3MB	+5.3KB
`inputControlVis`	49.9KB	49.9KB	+14.0B
`ml`	3.4MB	3.4MB	+53.0B
`securitySolution`	12.6MB	12.6MB	+84.0B
`sessionView`	385.0KB	384.8KB	-174.0B
`synthetics`	1.3MB	1.3MB	-11.0B
`visTypeVega`	1.8MB	1.8MB	+5.3KB
total			+10.6KB

Page load bundle

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

id	before	after	diff
`kbnUiSharedDeps-css`	340.5KB	324.3KB	-16.2KB
`kbnUiSharedDeps-npmDll`	5.2MB	5.3MB	+147.1KB
total			+130.9KB

Unknown metric groups

ESLint disabled in files

id	before	after	diff
`osquery`	1	2	+1

ESLint disabled line counts

id	before	after	diff
`enterpriseSearch`	19	21	+2
`fleet`	61	67	+6
`osquery`	109	115	+6
`securitySolution`	439	445	+6
total			+20

Total ESLint disabled count

id	before	after	diff
`enterpriseSearch`	20	22	+2
`fleet`	70	76	+6
`osquery`	110	117	+7
`securitySolution`	516	522	+6
total			+21

History

💔 Build #95774 failed cd832e6
💚 Build #94922 succeeded 21eaba4
💚 Build #94722 succeeded 56e45b1
💔 Build #94577 failed d950d8e
💔 Build #94554 failed 6c73e40

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

cc @breehall

Upgrade EUI to version 71.0.0

89c139a

breehall added release_note:skip Skip the PR/issue when compiling release notes EUI backport:skip This commit does not require backporting v8.7.0 labels Dec 6, 2022

cee-chen added 5 commits December 6, 2022 13:18

Remove EuiRangeLevel color overrides

996c31a

- the styling is significantly different on EuiRangeLevels and any previous tweaks likely should not carry over - also these classes no longer exist and as such their selectors no longer apply

[EuiRange] Fix multiple type imports to point to top-level EUI package

5a99109

[EuiDualRange] Update typing to account for now-required min/max props

5fffc5c

[EuiRange] Fix onChange typing

6f1dfd2

cee-chen force-pushed the eui-71.0.0 branch from 078d1fd to 5c575f9 Compare December 6, 2022 23:35

breehall added 5 commits December 7, 2022 13:18

[EuiRange] Updated EuiRange snapshots as it was recently converted to…

4578f14

… Emotion

[EuiBreadcrumb] Updated EuiBreadcrumb snapshots as the data-test-subj…

107bc37

… attribute was added in PR #6445

[Elastic Charts] Updated snapshots for donut color charts

45e329b

[EuiText] There was no visual or functional change. There was a chang…

408a33d

…e in the EuiText class name that required a snapshot update

[EuiText] There was no visual or functional change. There was a chang…

0ea2bd9

…e in the EuiText class name that required a snapshots update. [ScatterPlots] Updated the grayboilder plate and ticks/grid/axis colors within the ML scatterplt matrix wizard to match the EUI darkest shade

cee-chen assigned breehall Dec 13, 2022

breehall added 3 commits December 13, 2022 13:06

[EuiRange] Update onChange simulation to use currentTarget to obtain …

4c1ef08

…the range slider value

Merge main and resolve merge conflicts

50e5962

Removed conditional causing incorrect form state

6c73e40

kibanamachine and others added 3 commits December 13, 2022 19:18

Merge branch 'main' into eui-71.0.0

d950d8e

[ScatterPlot] Correct tolerance percentage on scatterPlotMatrixColors…

978c60a

…Wizard

Merge branch 'eui-71.0.0' of https://github.com/elastic/kibana into e…

56e45b1

…ui-71.0.0 Merging in latest code from EUI upgrade branch

breehall commented Dec 14, 2022

View reviewed changes

breehall marked this pull request as ready for review December 14, 2022 16:20

breehall requested review from a team as code owners December 14, 2022 16:20

breehall requested a review from a team December 14, 2022 16:20

breehall requested a review from banderror December 14, 2022 16:20

shahzad31 approved these changes Dec 14, 2022

View reviewed changes

botelastic bot added the Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability label Dec 14, 2022

patrykkopycinski approved these changes Dec 14, 2022

View reviewed changes

thomheymann approved these changes Dec 14, 2022

View reviewed changes

alisonelizabeth approved these changes Dec 14, 2022

View reviewed changes

nreese reviewed Dec 14, 2022

View reviewed changes

jbudz approved these changes Dec 14, 2022

View reviewed changes

TattdCodeMonkey approved these changes Dec 14, 2022

View reviewed changes

ThomThomson approved these changes Dec 14, 2022

View reviewed changes

pgayvallet approved these changes Dec 15, 2022

View reviewed changes

Merge branch 'main' into eui-71.0.0

21eaba4

stratoula approved these changes Dec 15, 2022

View reviewed changes

sphilipse approved these changes Dec 15, 2022

View reviewed changes

rshen91 approved these changes Dec 16, 2022

View reviewed changes

spong approved these changes Dec 16, 2022

View reviewed changes

vadimkibana approved these changes Dec 19, 2022

View reviewed changes

banderror removed their request for review December 19, 2022 08:38

michaelolo24 approved these changes Dec 19, 2022

View reviewed changes

peteharverson approved these changes Dec 19, 2022

View reviewed changes

Merge branch 'main' into eui-71.0.0

cd832e6

opauloh approved these changes Dec 19, 2022

View reviewed changes

Remove reference to removed EUI var

985f451

- use a more generic version instead

jbudz merged commit 6bdfc48 into main Dec 19, 2022

jbudz deleted the eui-71.0.0 branch December 19, 2022 21:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bumping EUI to 71.0.0 #147142

Bumping EUI to 71.0.0 #147142

breehall commented Dec 6, 2022

cee-chen commented Dec 14, 2022

breehall Dec 14, 2022

elasticmachine commented Dec 14, 2022

elasticmachine commented Dec 14, 2022

patrykkopycinski left a comment

thomheymann left a comment

alisonelizabeth left a comment

nreese Dec 14, 2022

cee-chen Dec 14, 2022 •

edited

Loading

ThomThomson left a comment

ThomThomson Dec 14, 2022

ThomThomson Dec 14, 2022

cee-chen Dec 14, 2022 •

edited

Loading

ThomThomson Dec 14, 2022

ThomThomson Dec 14, 2022

cee-chen Dec 14, 2022 •

edited

Loading

stratoula left a comment

sphilipse left a comment

spong left a comment

michaelolo24 left a comment

peteharverson left a comment

cee-chen commented Dec 19, 2022

opauloh left a comment

kibana-ci commented Dec 19, 2022

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

Bumping EUI to 71.0.0 #147142

Bumping EUI to 71.0.0 #147142

Conversation

breehall commented Dec 6, 2022

71.0.0

cee-chen commented Dec 14, 2022

breehall Dec 14, 2022

Choose a reason for hiding this comment

elasticmachine commented Dec 14, 2022

elasticmachine commented Dec 14, 2022

patrykkopycinski left a comment

Choose a reason for hiding this comment

thomheymann left a comment

Choose a reason for hiding this comment

alisonelizabeth left a comment

Choose a reason for hiding this comment

nreese Dec 14, 2022

Choose a reason for hiding this comment

cee-chen Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

ThomThomson left a comment

Choose a reason for hiding this comment

ThomThomson Dec 14, 2022

Choose a reason for hiding this comment

ThomThomson Dec 14, 2022

Choose a reason for hiding this comment

cee-chen Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

ThomThomson Dec 14, 2022

Choose a reason for hiding this comment

ThomThomson Dec 14, 2022

Choose a reason for hiding this comment

cee-chen Dec 14, 2022 • edited Loading

Choose a reason for hiding this comment

stratoula left a comment

Choose a reason for hiding this comment

sphilipse left a comment

Choose a reason for hiding this comment

spong left a comment

Choose a reason for hiding this comment

michaelolo24 left a comment

Choose a reason for hiding this comment

peteharverson left a comment

Choose a reason for hiding this comment

cee-chen commented Dec 19, 2022

opauloh left a comment

Choose a reason for hiding this comment

kibana-ci commented Dec 19, 2022

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Module Count

Async chunks

Page load bundle

ESLint disabled in files

ESLint disabled line counts

Total ESLint disabled count

History

`71.0.0`

cee-chen Dec 14, 2022 •

edited

Loading

cee-chen Dec 14, 2022 •

edited

Loading

cee-chen Dec 14, 2022 •

edited

Loading