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

Upgrade EUI to v31.7.0 #91210

Merged
merged 20 commits into from
Feb 16, 2021
Merged

Upgrade EUI to v31.7.0 #91210

merged 20 commits into from
Feb 16, 2021

Conversation

thompsongl
Copy link
Contributor

@thompsongl thompsongl commented Feb 11, 2021

eui@31.4.0eui@31.7.0

  • EuiOverlayMask is now included in EuiModal and EuiConfirmModal

31.7.0

  • Added whiteSpace prop to EuiCodeBlock. (#4475)
  • Added a light background to EuiDataGrid and removed unnecessary height on its container (#4509)

Bug fixes

  • Fixed bug in EuiDataGrid where the grid lost height when showing less rows on the last page (#4509)
  • Updated euiPaletteForStatus color sequence to use higher contrast postive and negative colors. (#4508)

31.6.0

  • Migrated dependency axe-puppeteer v1.1.1 to @axe-core/puppeteer v4.1.1 (#4482)
  • Added EuiOverlayMask directly to EuiModal (#4480)
  • Added paddingSize prop to EuiFlyout (#4448)
  • Added size='l' prop to EuiTabs (#4501)
  • Added content-specific props (pageTitle, description, tabs, rightSideItems) to EuiPageHeader by creating a new EuiPageHeaderContent component (#4451)
  • Added isActive parameter to the useIsWithinBreakpoints hook (#4451)
  • Added buttonProps prop to EuiAccordion (#4510)

Bug fixes

  • Fixed onClose invoking with unexpected parameter in EuiFlyout (#4505)
  • Fixed invalid color entry passed to EuiBadge color prop (#4481)
  • Fixed EuiCodeBlock focus-state if content overflows #4463
  • Fixed issues in EuiDataGrid around unnecessary scroll bars and container heights not updating (#4468)

Theme: Amsterdam

  • Increased EuiPage's default restrictWidth size to 1200px (extra large breakpoint) (#4451)
  • Reduced size of euiBottomShadowSmall by one layer (#4451)

31.5.0

  • Added isLoading prop and added EuiOverlayMask directly to EuiConfirmModal (#4421)
  • Added wrapWithTableRow to remove <tr> in EuiTableHeader(#4465)

Bug fixes

  • Fixed id usage throughout EuiTreeView to respect custom ids and stop conflicts in generated ids (#4435)
  • Fixed EuiTabs role if no tabs are passed in (#4435)
  • Fixed issue in EuiDataGrid where the horizontal scrollbar was hidden behind pagination (#4477)
  • Fixed EuiPopover with initial isOpen working with EuiOutsideClickDetector (#4461)
  • Fixed EuiDataGridCellPopover needing 2 state updates to close (#4461)
  • Fixed EuiBadge with iconOnClick from catching form submit events (#4479)

@thompsongl thompsongl added v8.0.0 release_note:skip Skip the PR/issue when compiling release notes v7.12.0 labels Feb 11, 2021
@thompsongl thompsongl changed the title Upgrade EUI to v31.6.0 Upgrade EUI to v31.7.0 Feb 12, 2021
@thompsongl thompsongl marked this pull request as ready for review February 12, 2021 18:15
@thompsongl thompsongl requested a review from a team as a code owner February 12, 2021 18:15
@thompsongl thompsongl requested review from a team February 12, 2021 18:15
@thompsongl thompsongl requested review from a team as code owners February 12, 2021 18:15
@thompsongl thompsongl requested a review from a team February 12, 2021 18:15
@thompsongl thompsongl requested review from a team as code owners February 12, 2021 18:15
Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

Data grid is adding a light shade of grey as background now:
Screenshot 2021-02-15 at 11 55 59

This is very distracting during virtualized scrolling because the grey flickers before it's replaced with new cells. What would be the best way to get the background back to white? The .euiDataGrid__content element is the issue here so we can't overwrite it from the outside without reaching into the EUI element structure which I would like to avoid if possible (if it's not, we can simply do that, it's not a big issue)

Copy link
Contributor

@YulNaumenko YulNaumenko left a comment

Choose a reason for hiding this comment

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

Alerting changes LGTM

@thompsongl
Copy link
Contributor Author

@azasypkin Can you provide more environment info? I cannot reproduce the issue you're seeing on this branch:

Screen Shot 2021-02-15 at 9 43 57 AM

@azasypkin
Copy link
Member

azasypkin commented Feb 15, 2021

@azasypkin Can you provide more environment info? I cannot reproduce the issue you're seeing on this branch:

Hmm, let me double check that I didn't mess up with anything.

@thompsongl
Copy link
Contributor Author

Data grid is adding a light shade of grey as background now

Deferring to @chandlerprall and @snide for what to do here. What do you two think?

Copy link
Member

@kertal kertal left a comment

Choose a reason for hiding this comment

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

Discover rightmost column scrolling overlapping issue was fixed ! 🥳
Discover_-_Elastic

@chandlerprall
Copy link
Contributor

Data grid is adding a light shade of grey as background now

Deferring to @chandlerprall and @snide for what to do here. What do you two think?

@flash1293 @thompsongl it is intentional, see https://github.com/elastic/eui/pull/4509/files#diff-5826316bba7a42b0d9228a47cdc8c8e67ed3eee9d092982cd5c4292258b7bc4fR36

@snide
Copy link
Contributor

snide commented Feb 15, 2021

The background color was intentional and arrived at after feedback in elastic/eui#4170 (review).

I wouldn't hold up this PR over it since it's a minor styling issue that can be changed with a one-liner on either side of the code. Likely we've got some back and forth with subjective opinions till everyone is happy, and that won't happen before tomorrow.

Ultimately, the heart of the problem is that I think we need a better concept of loading state with cells that are virtualized. That's definitely harder and will take another minor to land.

Copy link
Member

@azasypkin azasypkin 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! No idea what was wrong with my environment, but after yarn kbn clean and re-bootstrapping everything seems to be in order, sorry for the noise.

@weltenwort weltenwort self-requested a review February 15, 2021 19:50
@nchaulet
Copy link
Member

nchaulet commented Feb 15, 2021

for the fleet part looks like it's breaking our modal

Actyally it worked after running a clean before bootstraping again

Copy link
Member

@nchaulet nchaulet left a comment

Choose a reason for hiding this comment

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

Fleet changes 🚀

Copy link
Member

@weltenwort weltenwort left a comment

Choose a reason for hiding this comment

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

infra plugin changes LGTM 👍

(had to clean as well to get it working, though)

Copy link
Contributor

@flash1293 flash1293 left a comment

Choose a reason for hiding this comment

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

If the datagrid coloring got discussed and is the outcome of a deliberate decision, I'm happy with the current status, we can iterate on this.

What would be cool is to use a repeating background image of the cell borders so it's just "filling in" the content during scrolling.

Copy link
Contributor

@walterra walterra left a comment

Choose a reason for hiding this comment

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

ML changes LGTM.

Copy link
Contributor

@ThomThomson ThomThomson left a comment

Choose a reason for hiding this comment

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

Code only review, everything makes sense. Presentation Team changes LGTM

Copy link
Contributor

@smith smith left a comment

Choose a reason for hiding this comment

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

APM changes look good.

tenor-185243093

Copy link
Member

@cee-chen cee-chen left a comment

Choose a reason for hiding this comment

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

Huge thanks from the App Search team!

Copy link
Contributor

@alisonelizabeth alisonelizabeth left a comment

Choose a reason for hiding this comment

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

ES UI changes LGTM. Left one nit regarding removing use of fragment.

@kibanamachine
Copy link
Contributor

💚 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
apm 5.2MB 5.2MB -112.0B
beatsManagement 538.7KB 538.5KB -224.0B
canvas 1.4MB 1.4MB -849.0B
console 934.3KB 934.2KB -112.0B
crossClusterReplication 318.1KB 317.6KB -560.0B
dashboard 178.4KB 178.3KB -112.0B
data 236.5KB 236.3KB -224.0B
dataEnhanced 161.7KB 161.5KB -224.0B
enterpriseSearch 1.9MB 1.9MB -1.2KB
fleet 755.2KB 754.3KB -1008.0B
indexLifecycleManagement 240.2KB 240.0KB -224.0B
indexManagement 1.4MB 1.4MB -1.1KB
indexPatternManagement 556.0KB 555.7KB -336.0B
infra 1.9MB 1.9MB -448.0B
ingestPipelines 723.3KB 722.7KB -583.0B
licenseManagement 134.7KB 134.4KB -336.0B
logstash 48.8KB 48.6KB -224.0B
maps 2.6MB 2.6MB -112.0B
ml 6.3MB 6.3MB -2.8KB
observability 187.7KB 187.6KB -65.0B
remoteClusters 176.0KB 176.0KB -5.0B
reporting 34.8KB 34.7KB -112.0B
rollup 245.5KB 245.4KB -112.0B
savedObjectsManagement 164.1KB 163.8KB -336.0B
security 724.9KB 723.8KB -1.1KB
securitySolution 7.5MB 7.5MB -1.4KB
snapshotRestore 482.6KB 481.9KB -698.0B
transform 915.8KB 915.3KB -448.0B
triggersActionsUi 1.4MB 1.4MB -570.0B
uptime 926.4KB 926.2KB -224.0B
visualizations 56.9KB 56.8KB -112.0B
watcher 887.5KB 887.2KB -224.0B
total -16.0KB

Page load bundle

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

id before after diff
core 478.1KB 477.9KB -224.0B
indexLifecycleManagement 63.9KB 63.6KB -336.0B
kibanaReact 127.3KB 127.2KB -112.0B
maps 140.5KB 140.4KB -65.0B
savedObjects 56.0KB 55.9KB -112.0B
spaces 274.9KB 274.7KB -224.0B
total -1.0KB
Unknown metric groups

@kbn/ui-shared-deps asset size

id before after diff
css 586.6KB 590.4KB +3.8KB
kbn-ui-shared-deps.@elastic.js 2.5MB 2.5MB +6.1KB
kbn-ui-shared-deps.js 7.4MB 7.4MB +3.0B
total +9.9KB

History

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

@thompsongl thompsongl merged commit 8126488 into elastic:master Feb 16, 2021
thompsongl added a commit to thompsongl/kibana that referenced this pull request Feb 16, 2021
* eui to 31.6.0

* flyout, collapsible snapshot updates

* initial overlaymask removal

* undo jest

* overlaymask src snapshot updates

* more overlaymask removals

* overlaymask removal xpack test updates

* saved objects modal form

* eui to 31.7.0

* code, codeblock types

* snapshot update

* tooltip

* remove ownFocus from ConfirmModal

* remove fragments
# Conflicts:
#	x-pack/plugins/canvas/public/components/asset_manager/asset_manager.component.tsx
thompsongl added a commit that referenced this pull request Feb 16, 2021
* eui to 31.6.0

* flyout, collapsible snapshot updates

* initial overlaymask removal

* undo jest

* overlaymask src snapshot updates

* more overlaymask removals

* overlaymask removal xpack test updates

* saved objects modal form

* eui to 31.7.0

* code, codeblock types

* snapshot update

* tooltip

* remove ownFocus from ConfirmModal

* remove fragments
# Conflicts:
#	x-pack/plugins/canvas/public/components/asset_manager/asset_manager.component.tsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EUI release_note:skip Skip the PR/issue when compiling release notes Team:APM All issues that need APM UI Team support Team:Fleet Team label for Observability Data Collection Fleet team Team:Uptime - DEPRECATED Synthetics & RUM sub-team of Application Observability v7.12.0 v8.0.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.