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.

Sign up for GitHub

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

Jump to bottom

Upgrade EUI@13.3.0 #42858

Merged
merged 9 commits into from
Aug 20, 2019
Merged

Upgrade EUI@13.3.0 #42858

merged 9 commits into from
Aug 20, 2019

Conversation

snide
Copy link
Contributor

@snide snide commented Aug 7, 2019

Summary

Upgrades from 13.1.1. For outside reviews, updates are only to snap files and some i18n work.

13.3.0

  • Added i18n tokens to EuiSuperDatePicker and EuiSuperUpdateButton

13.2.0

  • Converted EuiStep, EuiSteps, EuiStepHorizontal, EuiStepsHorizontal, and EuiSubSteps to Typescript (#2186)

Bug fixes

  • Fixed EuiBadge truncation and auto-applied title attribute with innerText (#2190)
  • Remove exported TypeScript type and interface exports from built artifacts when they originate from node_modules (#2191)
  • Fixed EuiBadge truncation in IE and for the global filters pattern (#2194)
  • Fixed alignment of long titles in EuiStep (#2186)
  • Fixed the TS defs for EuiFilterSelectItem (#2192)
  • Added missing TS defs for EuiTextArea (#2201)

@snide snide requested a review from a team as a code owner August 7, 2019 18:41
@snide
Copy link
Contributor Author

snide commented Aug 7, 2019

cc @bmcconaghy Feel free to push against this as you need. @chandlerprall looks like some TS conversions, but snaps ran fine.

chandlerprall
chandlerprall approved these changes Aug 7, 2019
View reviewed changes
Copy link
Contributor

@chandlerprall chandlerprall left a comment

Choose a reason for hiding this comment

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

package, snapshot, and yarn.lock changes LGTM

@snide snide requested a review from a team as a code owner August 7, 2019 18:45
@snide snide added release_note:skip Skip the PR/issue when compiling release notes v8.0.0 v7.4.0 and removed release_note:skip Skip the PR/issue when compiling release notes labels Aug 7, 2019
@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@snide
Copy link
Contributor Author

snide commented Aug 7, 2019 •
edited
Loading

jenkins test this

@snide
Copy link
Contributor Author

snide commented Aug 7, 2019

jenkins retest

@elasticmachine
Copy link
Contributor

💔 Build Failed

mshustov
mshustov reviewed Aug 8, 2019
View reviewed changes
src/core/public/i18n/__snapshots__/i18n_service.test.tsx.snap
"euiSuperSelect.screenReaderAnnouncement": [Function],
"euiSuperSelectControl.selectAnOption": [Function],
"euiSuperUpdateButton.cannotUpdateTooltip": "Cannot update",
Copy link
Contributor

@mshustov mshustov Aug 8, 2019 •
edited
Loading

Choose a reason for hiding this comment

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

@elastic/kibana-platform do we need this test? testing translation format is not a task for service test. we can check that some keys are present in the mapping object.

@bmcconaghy
Copy link
Contributor

@restrry mind taking another look?

@elasticmachine
Copy link
Contributor

💔 Build Failed

@elasticmachine
Copy link
Contributor

💔 Build Failed

@chandlerprall
Copy link
Contributor

@spalger is investigating the CI failure. It appears that the Fixed EuiBadge truncation and auto-applied title attribute with innerText bugfix exposed some dependency misconfiguration in the CI build/test setup.

@bmcconaghy
Copy link
Contributor

@spalger any progress on this? I would like to get this in before FF for 7.4 as it includes a localization fix for the timepicker.

@snide
Copy link
Contributor Author

snide commented Aug 12, 2019

Reminder that chandler will be out this week on vacation. @thompsongl can help out on the EUI side @spalger.

@thompsongl
Copy link
Contributor

Anything I can do to help here, @spalger?

@spalger
Copy link
Contributor

spalger commented Aug 14, 2019

Sorry folks, github notifications have snowballed a bit, taking a look at this now

@spalger
Copy link
Contributor

spalger commented Aug 14, 2019

Alright, the issue is that the plugins in test/plugin_functional/plugins are pulling in the React install from the Kibana source, and all the rest of Kibana is using the React install from the distributable, which is an issue because we can't have multiple versions of React on the same page.

We can probably fix this by copying the testing plugins out of the Kibana directory and into the installation, but that's going to take some changes to @kbn/test. I'll see what I can do about getting those changes in.

@thompsongl
Copy link
Contributor

Multiple versions of React is what I suspected from the error, but it seems crazy we haven't hit this yet given we've been using hooks for months now.

Thanks, @spalger

@joshdover
Copy link
Contributor

@spalger Need any help on this? EUI has been bumped to 13.4.0 which I'd like to get in to solve a problem for the AppService PR (#41007)

@spalger spalger force-pushed the eui/13.3.0 branch 2 times, most recently from 919e99a to a6afbee Compare August 20, 2019 18:57
@elasticmachine

This comment has been minimized.

Sign in to view
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

spalger
spalger approved these changes Aug 20, 2019
View reviewed changes
Copy link
Contributor

@spalger spalger left a comment

Choose a reason for hiding this comment

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

LGTM

@snide snide merged commit b0a4181 into elastic:master Aug 20, 2019
@snide snide deleted the eui/13.3.0 branch August 20, 2019 20:37
@snide snide mentioned this pull request Aug 20, 2019
Merged
snide added a commit to snide/kibana that referenced this pull request Aug 20, 2019
@snide
Upgrade EUI@13.3.0 (elastic#42858)
571bfa2 
Upgrades 13.3.0
snide added a commit that referenced this pull request Aug 20, 2019
@snide
Upgrade EUI@13.3.0 (#42858) (#43613)
905ab26 
Upgrades 13.3.0
@chrisronline chrisronline mentioned this pull request Aug 21, 2019
Closed
@snide snide mentioned this pull request Aug 22, 2019
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bmcconaghy bmcconaghy bmcconaghy left review comments

@mshustov mshustov mshustov approved these changes

@spalger spalger spalger approved these changes

@chandlerprall chandlerprall chandlerprall approved these changes

@sorenlouv sorenlouv sorenlouv approved these changes

Assignees
No one assigned
Labels
release_note:skip Skip the PR/issue when compiling release notes v7.4.0 v8.0.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
9 participants
@snide @elasticmachine @bmcconaghy @chandlerprall @thompsongl @spalger @joshdover @sorenlouv @mshustov