-
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
[Visual Refresh] Add Borealis theme #199993
Conversation
1cc9b49
to
b33a410
Compare
🤖 GitHub commentsExpand to view the GitHub comments
Just comment with:
|
cb7491a
to
00df65d
Compare
4f4198f
to
a9c14b6
Compare
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.
DW changes ✅
@@ -8,25 +8,44 @@ | |||
*/ | |||
|
|||
import React, { FC } from 'react'; | |||
import type { DarkModeValue } from '@kbn/core-ui-settings-common'; | |||
import { type DarkModeValue, ThemeName } from '@kbn/core-ui-settings-common'; |
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.
The type of ThemeName
is now string
but will get stricter in follow-up PRs
@@ -426,8 +426,8 @@ export default ({ getPageObjects, getService }: FtrProviderContext) => { | |||
expect(nodesWithValue).to.eql([ | |||
{ name: 'host-5', value: 10, color: '#6092c0' }, | |||
{ name: 'host-4', value: 30, color: '#9ab6d5' }, | |||
{ name: 'host-1', value: 50, color: '#f1d9b9' }, | |||
{ name: 'host-2', value: 70, color: '#eba47a' }, | |||
{ name: 'host-1', value: 50, color: '#f6e0b9' }, |
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.
These values change because we updated how colors are calculated. The changes should be pretty much indistinguishable.
#f1d9b9
->#f6e0b9
#eba47a
->#eda77a
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 doing this, was actually on my list of things we needed to look into with the change of the theme
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.
Core changes LGTM
packages/core/rendering/core-rendering-server-internal/src/views/styles.tsx
Outdated
Show resolved
Hide resolved
FYI I'm investigating the increased bundle size |
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.
Left one curiosity question, but otherwise this LGTM. Assuming there is a good reason to hardcode those color values I'm asking about, I'll approve now to not hold you up.
packages/core/rendering/core-rendering-server-internal/src/views/styles.tsx
Show resolved
Hide resolved
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.
⏳ Build in-progress
History
|
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.
LGTM! I added a comment to create an issue to not lose track of the work to be done when Borealis is in public beta.
Approving to unblock you but it would be great to have it. Cheers 👍
@@ -52,6 +52,11 @@ export function parseThemeTags(input?: unknown): ThemeTags { | |||
return DEFAULT_THEME_TAGS; | |||
} | |||
|
|||
// TODO: remove when Borealis is in public beta |
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.
Can we create an issue for this to not forget to remove this condition? And then update the comment with the link to the issue?
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.
Surely! I'm going to create an issue shortly and will update the comment in a follow-up PR so that we don't need to wait for CI to pass again. Thanks for catching it!
This reverts commit ee49986.
This reverts commit ee49986.
## Summary This PR introduces the first internal version of the new theme `Borealis` and ensures that: - themes can be switched between "Amsterdam" and "Borealis" - theme-specific Sass files are available and can be loaded with `KBN_OPTIMIZER_THEMES=experimental` - legacy JSON variable usage accounts for both themes - static template styles account for both themes ## Running locally ```yml // kibana.dev.yml or kibana.yml uiSettings.experimental.themeSwitcherEnabled: true ``` Start kibana ``` KBN_OPTIMIZER_THEMES='v8light,v8dark,borealislight,borealisdark' yarn start or KBN_OPTIMIZER_THEMES=experimental yarn start ``` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed --------- Co-authored-by: Tomasz Kajtoch <[email protected]>
## Summary This PR introduces the first internal version of the new theme `Borealis` and ensures that: - themes can be switched between "Amsterdam" and "Borealis" - theme-specific Sass files are available and can be loaded with `KBN_OPTIMIZER_THEMES=experimental` - legacy JSON variable usage accounts for both themes - static template styles account for both themes ## Running locally ```yml // kibana.dev.yml or kibana.yml uiSettings.experimental.themeSwitcherEnabled: true ``` Start kibana ``` KBN_OPTIMIZER_THEMES='v8light,v8dark,borealislight,borealisdark' yarn start or KBN_OPTIMIZER_THEMES=experimental yarn start ``` ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed --------- Co-authored-by: Tomasz Kajtoch <[email protected]>
Summary
This PR introduces the first internal version of the new theme
Borealis
and ensures that:KBN_OPTIMIZER_THEMES=experimental
Running locally
Start kibana
Checklist