-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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 v43.1.1 #120935
Upgrade EUI to v43.1.1 #120935
Changes from 10 commits
3d27dde
ae5276c
c233f80
9e49e8f
d73dbee
138ab58
72de6ee
4ef571f
8c67812
0a4c8ed
1460999
655ab86
af37e56
d97e9fd
7f85b10
5e4a764
f370b20
25d8b6b
9b530e0
719c5a8
104ba53
4616bc7
1e9a278
3f10425
5d514cc
35705e1
3a33df2
0f0ffb3
0ea0e51
04db019
253e475
2e555b5
c179517
d67dd95
cc0b540
b579d27
db050f0
9d0a26e
07eee93
5083a50
d5b68ce
75f7ac4
dc71e80
b582282
69a308a
b4c2ab7
a280d69
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,7 +9,9 @@ | |
import React, { FC, useMemo } from 'react'; | ||
import { Observable } from 'rxjs'; | ||
import useObservable from 'react-use/lib/useObservable'; | ||
import { EuiThemeProvider } from '@elastic/eui'; | ||
import createCache from '@emotion/cache'; | ||
import { EuiProvider } from '@elastic/eui'; | ||
import { EUI_STYLES_GLOBAL } from '../../utils'; | ||
import { CoreTheme } from './types'; | ||
import { convertCoreTheme } from './convert_core_theme'; | ||
|
||
|
@@ -21,16 +23,26 @@ interface CoreThemeProviderProps { | |
theme$: Observable<CoreTheme>; | ||
} | ||
|
||
const emotionCache = createCache({ | ||
key: 'eui-styles', | ||
container: document.querySelector(`meta[name="${EUI_STYLES_GLOBAL}"]`) as HTMLElement, | ||
}); | ||
Comment on lines
+26
to
+29
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum, shouldn't this also be used in the public version of the EuiProvider wrapper in If that's the case, we'll have to expose this cache from There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Only the top-level theme provider needs the cache because it's the only provider that now also provides global styles. Those global styles only need to be included once and are the only styles that require the cache. Eventually, once EUI has converted all components to Emotion, the global style cache will be unnecessary and can be removed. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ok, Just to be sure we're on the same page, this It is used as a 'top level' provider for chrome's react tree, but is also used for internal core's components, such as toasts, flyout and modal, as all those components are their own distinct react tree / ReactDOM.render call. Is it still fine? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is fine, thanks for clarifying. My main point was that a nested KibanaThemeProvider does not need to be aware of the cache. I'll probably have a follow-up PR that ensures no duplication of the global styles from the various toasts, flyout and modal, etc. instances, but there will be no adverse affects in the meantime. |
||
|
||
/** | ||
* Wrapper around `EuiThemeProvider` converting (and exposing) core's theme to EUI theme. | ||
* Wrapper around `EuiProvider` converting (and exposing) core's theme to EUI theme. | ||
* @internal Only meant to be used within core for internal usages of EUI/React | ||
*/ | ||
export const CoreThemeProvider: FC<CoreThemeProviderProps> = ({ theme$, children }) => { | ||
const coreTheme = useObservable(theme$, defaultTheme); | ||
const euiTheme = useMemo(() => convertCoreTheme(coreTheme), [coreTheme]); | ||
return ( | ||
<EuiThemeProvider colorMode={euiTheme.colorMode} theme={euiTheme.euiThemeSystem}> | ||
<EuiProvider | ||
modify={{ LIGHT: { colors: { primary: '#FF0000' } } }} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think declaring a const for this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for flagging this. This was meant entirely to test the setup and will be removed before merging. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will want to expand the API to pass |
||
colorMode={euiTheme.colorMode} | ||
theme={euiTheme.euiThemeSystem} | ||
cache={emotionCache} | ||
> | ||
{children} | ||
</EuiThemeProvider> | ||
</EuiProvider> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
/* | ||
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
* or more contributor license agreements. Licensed under the Elastic License | ||
* 2.0 and the Server Side Public License, v 1; you may not use this file except | ||
* in compliance with, at your election, the Elastic License 2.0 or the Server | ||
* Side Public License, v 1. | ||
*/ | ||
|
||
export const EUI_STYLES_GLOBAL = 'eui-styles-global'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,6 +76,6 @@ export const LICENSE_OVERRIDES = { | |
'[email protected]': ['Eclipse Distribution License - v 1.0'], // cf. https://github.com/bjornharrtell/jsts | ||
'@mapbox/[email protected]': ['MIT'], // license in readme https://github.com/tmcw/jsonlint | ||
'@elastic/[email protected]': ['Elastic License 2.0'], | ||
'@elastic/eui@41.2.3': ['SSPL-1.0 OR Elastic License 2.0'], | ||
'@elastic/eui@43.0.0': ['SSPL-1.0 OR Elastic License 2.0'], | ||
'[email protected]': ['CC-BY-4.0'], // retired ODC‑By license https://github.com/mattcg/language-subtag-registry | ||
}; |
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.
👍 thanks for that