Fix discover, tsvb and Lens chart theming issues by nickofthyme · Pull Request #69695 · elastic/kibana

nickofthyme · 2020-06-23T15:04:36Z

Summary

Fixes #69667 and #70181

Updates TSVB and discover charts to use baseTheme from charts plugin theme service.
Updates TSVB to handle custom background colors per changes from feat: add 4.5 contrast for text in partition slices elastic-charts#608
Updates theme service in the Charts plugin to include base theme.

Adds the following to theme service:

chartsDefaultBaseTheme
darkModeEnabled$ - observable for dark mode boolean
chartsBaseTheme$ - observable to get base theme based on dark mode
useChartsBaseTheme - React hook to get base theme based on dark mode

Screenshots

TSVB

Discover

Checklist

Documentation was added for features that require explanation or tutorials
This was checked for cross-browser compatibility, including a check against IE11

elasticmachine · 2020-06-23T15:04:39Z

Pinging @elastic/kibana-app (Team:KibanaApp)

cchaos · 2020-06-23T20:36:00Z

src/plugins/charts/public/services/theme/README.md

@@ -0,0 +1,92 @@
+# Theme Service
+
+The `theme` service offers utilities to interact with the kibana theme. EUI provides a light and dark theme object to suplement the Elastic-Charts `baseTheme`. However, every instance of a Chart would need to pass down this the correctly EUI theme depending on Kibana's light or dark mode. There are several ways you can use the `theme` service to get the correct shared `theme` and `baseTheme`.


Suggested change

The `theme` service offers utilities to interact with the kibana theme. EUI provides a light and dark theme object to suplement the Elastic-Charts `baseTheme`. However, every instance of a Chart would need to pass down this the correctly EUI theme depending on Kibana's light or dark mode. There are several ways you can use the `theme` service to get the correct shared `theme` and `baseTheme`.

The `theme` service offers utilities to interact with the kibana theme. EUI provides a light and dark theme object to supplement the Elastic-Charts `baseTheme`. However, every instance of a Chart would need to pass down the correct EUI theme depending on Kibana's light or dark mode. There are several ways you can use the `theme` service to get the correct shared `theme` and `baseTheme`.

Gracias 189843c

cchaos · 2020-06-23T20:37:07Z

src/plugins/charts/public/services/theme/README.md

+
+## Why have `theme` and `baseTheme`?
+
+The `theme` prop is a recursive partial `Theme` that overrides properties from the `baseTheme`. This allows changes to the `Theme` type in `@elastic/charts` without having to update the `@elastic/eui` themes or every `<Chart />`.


Suggested change

The `theme` prop is a recursive partial `Theme` that overrides properties from the `baseTheme`. This allows changes to the `Theme` type in `@elastic/charts` without having to update the `@elastic/eui` themes or every `<Chart />`.

The `theme` prop is a recursive partial `Theme` that overrides properties from the `baseTheme`. This allows changes to the `Theme` TS type in `@elastic/charts` without having to update the `@elastic/eui` themes for every `<Chart />`.

Does this mean that every <Chart /> instance now must provide both the baseTheme and the EUI provided theme?

It's not a must. But since we added the background color to the theme for the contrast ratio logic, the background color defaults to a non-transparent colors respective of the mode (i.e. light and dark). So in order for the chart to work in both dark and light modes without a baseTheme, one would need to set the theme.background.color to 'transparent'. This would avoid the contrast logic though.

I'm not sure if 'transparent' should just be the default for the background color or if you just want to add this to the EUI themes.

But on a more general note, I think it's helpful to have a baseTheme used in all elastic/charts Charts in kibana for times when we change the Theme object. This allows for nothing to break while in the meantime we update any changes to the EUI charts partial themes.

I raised the question in an EC issue, to see how we could do this better (see elastic/elastic-charts#718). As of now, I think a HOC to handle this logic and allow overrides would be the most practicable option. Something like...

const MyComponent = () => ( <Chart> <ThemedSettings theme={EuiThemeOverrides} {...otherSettingsProps} /> {/* more goodness */} </Chart> )

Fixed spelling errors here 189843c

cchaos · 2020-06-23T20:43:18Z

src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/index.js

+            },
+          },
+        ]}
+        baseTheme={baseTheme}


This looks to me like it no longer is getting the EUI theme?

I don't think TSVB was ever using the EUI charts theme. The original getTheme code below only depended on the @elastic/charts themes. This is pretty clear from the UI of TSVB compared to charts using EUI, EUI themed charts look much nicer! 😝

kibana/src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/utils/theme.ts

Lines 97 to 131 in e3d01bf

export function getTheme(darkMode: boolean, bgColor?: string | null): Theme {

if (!isValidColor(bgColor)) {

return darkMode ? DARK_THEME : LIGHT_THEME;

}

const bgLuminosity = computeRelativeLuminosity(bgColor);

const mainTheme = bgLuminosity <= 0.179 ? DARK_THEME : LIGHT_THEME;

const color = findBestContrastColor(

bgColor,

LIGHT_THEME.axes.axisTitleStyle.fill,

DARK_THEME.axes.axisTitleStyle.fill

);

return {

...mainTheme,

axes: {

...mainTheme.axes,

axisTitleStyle: {

...mainTheme.axes.axisTitleStyle,

fill: color,

},

tickLabelStyle: {

...mainTheme.axes.tickLabelStyle,

fill: color,

},

axisLineStyle: {

...mainTheme.axes.axisLineStyle,

stroke: color,

},

tickLineStyle: {

...mainTheme.axes.tickLineStyle,

stroke: color,

},

},

};

}

That said I think we could make TSVB use the EUI theme in another PR. It should be easy to merge the getTheme logic with the EUI themes.

cchaos · 2020-06-23T20:44:17Z

...s/vis_type_timeseries/public/application/visualizations/views/timeseries/utils/theme.test.ts

 */

-import { getTheme } from './theme';
+import { getBaseTheme } from './theme';


Are you trying to completely replace getTheme with getBaseTheme?

What do you mean by replace it?

This was just a utility function to get the theme which was actually generating a base theme so I just updated the naming.

Ideally, there is a baseTheme from @elastic/charts. Then this function returns a Partial<Theme> using EUI that passes the value to the Chart.theme prop.

YulNaumenko

Need to apply the proposed changes to fix crashing of our component which using Chart

...ers_actions_ui/public/application/components/builtin_alert_types/threshold/visualization.tsx

YulNaumenko

After changes, alerting code LGTM

stratoula · 2020-06-24T08:41:46Z

src/plugins/charts/README.md

@@ -26,72 +26,4 @@ Truncated color mappings in `value`/`text` form



It's irrelevant with this PR but I saw on L21 that there is a typo Funciton instead of Function and retrive instead of retrieve. Do you want to also correct it in this PR ?

Fixed in d82c6b8

stratoula · 2020-06-24T09:06:40Z

src/plugins/vis_type_timeseries/public/application/visualizations/views/timeseries/index.js

 };

 export const TimeSeries = ({
-  darkMode,


We should also remove the darkMode prop Type on L276

~~I removed the darkMode as it was only used to pass to the getTheme function to select the baseTheme from @elastic/charts which is what the useChartBaseTheme hook does on it's own.~~

Sorry misread that, yes I'll remove the propType, thanks!

fixed in d82c6b8

stratoula · 2020-06-24T09:30:05Z

@nickofthyme I noticed that while now the bg color change works as it should there is a problem when applying color with transparency. For example:

As you can see the border has the correct color but the main container is darker.

On 7.8 instance the background color appears smoothly

nickofthyme · 2020-06-24T14:03:35Z

@stratoula great find on the transparency. It looks like tsvb was applying the bgColor to the parent div and adding padding.

Since there is currently no way to padding around the entire chart via the @elastic/charts theme. I added a border with the bgColor to avoid overlapping alpha colors. When we have a way to set the external margins of the chart (including legends and all) we could simplify this styling.

See d82c6b8

cchaos

I guess I got really confused as to what the differences in chart theme services there are. There's a theme provider from Charts and one from EUI? Ack I dunno, super confused but so long as no one else is... 🤷

cchaos · 2020-06-24T19:04:07Z

src/plugins/vis_type_timeseries/public/application/components/vis_types/_vis_types.scss

-  padding: $euiSizeS;
+  border-width: $euiSizeS;
+  border-style: solid;
+  border-color: transparent;


I look at this and think "Why not padding?" Can you please add a comment here as to why it's necessary to do it this way so no one tries to optimize by converting it back to padding?

Yeah this is a workaround for this issue.

Bascially, the containing element for TSVB was applying a background color and padding. This was fine when elastic charts background was transparent but not anymore.

I think this can be cleaned up when we allow padding around the whole chart in the Theme but for now changing the theme.chartMargins does not include the legend.

Yeah no worries. Just suggesting adding a comment in the SASS file to keep others aware if they jump in this file later.

nickofthyme · 2020-06-25T13:45:59Z

@cchaos The basic idea is each chart should have a baseTheme (for light and dark) and an EUI theme (for light and dark). The EUI theme is only overriding some of the theme values. Without setting the baseTheme we automatically use our LIGHT_THEME, so even if you use the EUI dark theme there may be a property that is out of place because it's using a value from our LIGHT_THEME. The idea is that we can add things to the baseThemes as needed and later override these new properties in the EUI themes.

I can jump on a zoom call to explain more if you'd like.

cchaos

Thanks @nickofthyme, that makes sense to me. However, if EUI isn't changing some colors from the base theme it's probably not going to align with EUI colors. Can you send me the new theme keys that the EUI theme isn't addressing?

cchaos · 2020-06-25T14:30:48Z

src/plugins/vis_type_timeseries/public/application/components/vis_types/_vis_types.scss

-  padding: $euiSizeS;
+  border-width: $euiSizeS;
+  border-style: solid;
+  border-color: transparent;


Yeah no worries. Just suggesting adding a comment in the SASS file to keep others aware if they jump in this file later.

nickofthyme · 2020-06-25T14:42:47Z

Yeah I don't think there are a lot of changes but here is the EUI theme and here is the light and dark @elastic/charts themes.

nickofthyme · 2020-06-29T15:08:21Z

@flash1293 can you check on fb35a49 when you get a chance? BTW the baseTheme idea is explained here #69695 (comment). I opened an issue to make this nicer to share themes in the future elastic/elastic-charts#718.

flash1293 · 2020-06-30T06:01:14Z

@nickofthyme Seems like some unit tests have to be adjusted. Let me know when I can help here.

- fix broken jest tests in lens - update datapannel to use charts theme - update FieldsAccordion to use charts theme

nickofthyme · 2020-06-30T15:22:35Z

Thanks @flash1293 I updated tests and fixed the type errors

nickofthyme · 2020-07-01T17:15:35Z

@flash1293 or @wylieconlon I made some changes to Lens to use the charts plugin to grab the correct theme and baseTheme. I verified everything looks good in Lens but if one of you could do a quick pass that would be appreciated.

flash1293

Wow, thanks for fixing this in Lens as well, @nickofthyme - really appreciate the help.

I only have one nit (but If you want to merge this PR before fixing it, I'm fine with it as well)

In the pie chart, the outer labels don't have great contrast in dark mode:
This PR:

7.8:

nickofthyme · 2020-07-02T14:00:47Z

@flash1293 Thanks for taking a look.

Yeah this PR was supposed to enhance the color contrast for partition charts, however we ran into an issue where to do so we would need to set the background color on all charts in kibana.

For now, the background color is set to 'transparent' by default. Until all kibana charts use the charts plugin theme service.

I added the background color to the EUI charts theme here so the next version should automatically give you the ability to enhance the text contrast on the partition charts using a config like below. See elastic/elastic-charts#608

config: {
  fillLabel: {
    textInvertible: true,
    textContrast: true, // can also be set to a number
  }
}

* master: Update known-plugins.asciidoc (elastic#69370) [ML] Anomaly Explorer swim lane pagination (elastic#70063) [Ingest Manager] Update asset paths to use _ instead of - (elastic#70320) Fix discover, tsvb and Lens chart theming issues (elastic#69695) Allow Saved Object type mappings to set a field's `doc_values` property (elastic#70433) [S&R] Support data streams (elastic#68078) [Maps] Add styling and tooltip support to mapbox mvt vector tile sources (elastic#64488) redirect to default app if hash can not be forwarded (elastic#70417) [APM] Don't fetch dynamic index pattern in setupRequest (elastic#70308) [Security_Solution][Endpoint] Leveraging msearch and ancestry array for resolver (elastic#70134) Update docs for api authentication usage (elastic#66819) chore(NA): disable alerts_detection_rules cypress suites (elastic#70577) add getVisibleTypes API to SO type registry (elastic#70559)

kibanamachine · 2020-07-02T16:48:28Z

💔 Build Failed

continuous-integration/kibana-ci/pull-request
Commit: f5fcbf3
Pipeline Steps (look for red circles / failed steps)
Interpreting CI Failures

Failed CI Steps

Test Failures

Kibana Pipeline / kibana-xpack-agent / Chrome X-Pack UI Functional Tests.x-pack/test/functional/apps/index_management/home_page·ts.Index Management app Home page Component templates renders the component templates tab

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]       │
[00:33:48]         └-: Index Management app
[00:33:48]           └-> "before all" hook
[00:33:48]           └-: Home page
[00:33:48]             └-> "before all" hook
[00:33:48]             └-> "before all" hook
[00:33:48]               │ debg navigating to indexManagement url: http://localhost:6141/app/management/data/index_management
[00:33:48]               │ debg navigate to: http://localhost:6141/app/management/data/index_management
[00:33:48]               │ proc [kibana]   log   [16:10:41.185] [info][authentication][plugins][security] Authentication attempt failed: [security_exception] unable to authenticate user [ml_poweruser] for REST request [/_security/_authenticate], with { header={ WWW-Authenticate={ 0="ApiKey" & 1="Basic realm=\"security\" charset=\"UTF-8\"" } } }
[00:33:48]               │ERROR browser[SEVERE] http://localhost:6141/app/management/data/index_management?_t=1593706241126 - Failed to load resource: the server responded with a status of 401 (Unauthorized)
[00:33:48]               │ debg ... sleep(700) start
[00:33:48]               │ debg ... sleep(700) end
[00:33:48]               │ debg returned from get, calling refresh
[00:33:49]               │ debg browser[INFO] http://localhost:6141/login?next=%2Fapp%2Fmanagement%2Fdata%2Findex_management%3F_t%3D1593706241126 341 Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'unsafe-eval' 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-P5polb1UreUSOe5V/Pv7tc+yeZuJXiOi/3fqhGsU7BE='), or a nonce ('nonce-...') is required to enable inline execution.
[00:33:49]               │
[00:33:49]               │ debg browser[INFO] http://localhost:6141/bundles/app/core/bootstrap.js 43:19 "^ A single error about an inline script not firing due to content security policy is expected!"
[00:33:49]               │ debg currentUrl = http://localhost:6141/login?next=%2Fapp%2Fmanagement%2Fdata%2Findex_management%3F_t%3D1593706241126
[00:33:49]               │          appUrl = http://localhost:6141/app/management/data/index_management
[00:33:49]               │ debg TestSubjects.find(kibanaChrome)
[00:33:49]               │ debg Find.findByCssSelector('[data-test-subj="kibanaChrome"]') with timeout=60000
[00:33:50]               │ debg Found login page
[00:33:50]               │ debg TestSubjects.setValue(loginUsername, test_user)
[00:33:50]               │ debg TestSubjects.click(loginUsername)
[00:33:50]               │ debg Find.clickByCssSelector('[data-test-subj="loginUsername"]') with timeout=10000
[00:33:50]               │ debg Find.findByCssSelector('[data-test-subj="loginUsername"]') with timeout=10000
[00:33:50]               │ debg browser[INFO] http://localhost:6141/34234/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js 444:106112 "INFO: 2020-07-02T16:10:43Z
[00:33:50]               │        Adding connection to http://localhost:6141/elasticsearch
[00:33:50]               │
[00:33:50]               │      "
[00:33:51]               │ debg TestSubjects.setValue(loginPassword, changeme)
[00:33:51]               │ debg TestSubjects.click(loginPassword)
[00:33:51]               │ debg Find.clickByCssSelector('[data-test-subj="loginPassword"]') with timeout=10000
[00:33:51]               │ debg Find.findByCssSelector('[data-test-subj="loginPassword"]') with timeout=10000
[00:33:51]               │ debg TestSubjects.click(loginSubmit)
[00:33:51]               │ debg Find.clickByCssSelector('[data-test-subj="loginSubmit"]') with timeout=10000
[00:33:51]               │ debg Find.findByCssSelector('[data-test-subj="loginSubmit"]') with timeout=10000
[00:33:51]               │ debg Find.findByCssSelector('[data-test-subj="kibanaChrome"] nav:not(.ng-hide)') with timeout=60000
[00:33:53]               │ debg browser[INFO] http://localhost:6141/app/management/data/index_management?_t=1593706241126 341 Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'unsafe-eval' 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-P5polb1UreUSOe5V/Pv7tc+yeZuJXiOi/3fqhGsU7BE='), or a nonce ('nonce-...') is required to enable inline execution.
[00:33:53]               │
[00:33:53]               │ debg browser[INFO] http://localhost:6141/bundles/app/core/bootstrap.js 43:19 "^ A single error about an inline script not firing due to content security policy is expected!"
[00:33:53]               │ debg browser[INFO] http://localhost:6141/34234/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js 444:106112 "INFO: 2020-07-02T16:10:45Z
[00:33:53]               │        Adding connection to http://localhost:6141/elasticsearch
[00:33:53]               │
[00:33:53]               │      "
[00:33:53]               │ debg browser[INFO] http://localhost:6141/app/management/data/index_management?_t=1593706246452 341 Refused to execute inline script because it violates the following Content Security Policy directive: "script-src 'unsafe-eval' 'self'". Either the 'unsafe-inline' keyword, a hash ('sha256-P5polb1UreUSOe5V/Pv7tc+yeZuJXiOi/3fqhGsU7BE='), or a nonce ('nonce-...') is required to enable inline execution.
[00:33:53]               │
[00:33:53]               │ debg browser[INFO] http://localhost:6141/bundles/app/core/bootstrap.js 43:19 "^ A single error about an inline script not firing due to content security policy is expected!"
[00:33:53]               │ debg Finished login process currentUrl = http://localhost:6141/app/management/data/index_management
[00:33:53]               │ debg ... sleep(501) start
[00:33:54]               │ debg ... sleep(501) end
[00:33:54]               │ debg in navigateTo url = http://localhost:6141/app/management/data/index_management
[00:33:54]               │ debg TestSubjects.exists(statusPageContainer)
[00:33:54]               │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="statusPageContainer"]') with timeout=2500
[00:33:56]               │ debg browser[INFO] http://localhost:6141/34234/bundles/kbn-ui-shared-deps/kbn-ui-shared-deps.js 444:106112 "INFO: 2020-07-02T16:10:47Z
[00:33:56]               │        Adding connection to http://localhost:6141/elasticsearch
[00:33:56]               │
[00:33:56]               │      "
[00:33:56]               │ debg --- retry.tryForTime error: [data-test-subj="statusPageContainer"] is not displayed
[00:33:57]             └-> Loads the app and renders the indices tab by default
[00:33:57]               └-> "before each" hook: global before each
[00:33:57]               │ debg Checking for section heading to say Index Management.
[00:33:57]               │ debg TestSubjects.getVisibleText(appTitle)
[00:33:57]               │ debg TestSubjects.find(appTitle)
[00:33:57]               │ debg Find.findByCssSelector('[data-test-subj="appTitle"]') with timeout=10000
[00:33:57]               │ debg TestSubjects.exists(indicesList)
[00:33:57]               │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="indicesList"]') with timeout=2500
[00:33:57]               │ debg TestSubjects.find(reloadIndicesButton)
[00:33:57]               │ debg Find.findByCssSelector('[data-test-subj="reloadIndicesButton"]') with timeout=10000
[00:33:57]               └- ✓ pass  (98ms) "Index Management app Home page Loads the app and renders the indices tab by default"
[00:33:57]             └-: Data streams
[00:33:57]               └-> "before all" hook
[00:34:00]             └-: Component templates
[00:34:00]               └-> "before all" hook
[00:34:00]               └-> renders the component templates tab
[00:34:00]                 └-> "before each" hook: global before each
[00:34:00]                 │ debg TestSubjects.click(component_templatesTab)
[00:34:00]                 │ debg Find.clickByCssSelector('[data-test-subj="component_templatesTab"]') with timeout=10000
[00:34:00]                 │ debg Find.findByCssSelector('[data-test-subj="component_templatesTab"]') with timeout=10000
[00:34:00]                 │ debg isGlobalLoadingIndicatorVisible
[00:34:00]                 │ debg TestSubjects.exists(globalLoadingIndicator)
[00:34:00]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="globalLoadingIndicator"]') with timeout=1500
[00:34:00]                 │ debg TestSubjects.exists(globalLoadingIndicator-hidden)
[00:34:00]                 │ debg Find.existsByCssSelector('[data-test-subj="globalLoadingIndicator-hidden"]') with timeout=100000
[00:34:00]                 │ debg TestSubjects.exists(emptyList)
[00:34:00]                 │ debg Find.existsByDisplayedByCssSelector('[data-test-subj="emptyList"]') with timeout=2500
[00:34:03]                 │ debg --- retry.tryForTime error: [data-test-subj="emptyList"] is not displayed
[00:34:03]                 │ info Taking screenshot "/dev/shm/workspace/kibana/x-pack/test/functional/screenshots/failure/Index Management app Home page Component templates renders the component templates tab.png"
[00:34:03]                 │ info Current URL is: http://localhost:6141/app/management/data/index_management/component_templates
[00:34:03]                 │ info Saving page source to: /dev/shm/workspace/kibana/x-pack/test/functional/failure_debug/html/Index Management app Home page Component templates renders the component templates tab.html
[00:34:03]                 └- ✖ fail: "Index Management app Home page Component templates renders the component templates tab"
[00:34:03]                 │

Stack Trace

Error: expected false to equal true
    at Assertion.assert (/dev/shm/workspace/kibana/packages/kbn-expect/expect.js:100:11)
    at Assertion.be.Assertion.equal (/dev/shm/workspace/kibana/packages/kbn-expect/expect.js:227:8)
    at Assertion.be (/dev/shm/workspace/kibana/packages/kbn-expect/expect.js:69:22)
    at Context.it (test/functional/apps/index_management/home_page.ts:86:49)

Kibana Pipeline / kibana-xpack-agent / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/templates·js.apis management index management index templates get all should list all the index templates with the expected parameters

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has not failed recently on tracked branches

[00:00:00]       │
[00:00:00]         └-: apis
[00:00:00]           └-> "before all" hook
[00:04:25]           └-: management
[00:04:25]             └-> "before all" hook
[00:04:45]             └-: index management
[00:04:45]               └-> "before all" hook
[00:04:48]               └-: index templates
[00:04:48]                 └-> "before all" hook
[00:04:48]                 └-: get all
[00:04:48]                   └-> "before all" hook
[00:04:48]                   └-> should list all the index templates with the expected parameters
[00:04:48]                     └-> "before each" hook: global before each
[00:04:48]                     └-> "before each" hook
[00:04:48]                       │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xl-1593702143659934750] adding template [template-omqncyjzqphncifkgs-1593706462617] for index patterns [test*]
[00:04:48]                     └- ✖ fail: "apis management index management index templates get all should list all the index templates with the expected parameters"
[00:04:48]                     │

Stack Trace

{ Error: expected [ { name: 'metrics',
    version: 0,
    priority: 100,
    indexPatterns: [ 'metrics-*-*' ],
    composedOf: [ 'metrics-mappings', 'metrics-settings' ],
    _kbnMeta: { isManaged: false },
    hasSettings: false,
    hasAliases: false,
    hasMappings: false },
  { name: 'logs',
    version: 0,
    priority: 100,
    indexPatterns: [ 'logs-*-*' ],
    composedOf: [ 'logs-mappings', 'logs-settings' ],
    _kbnMeta: { isManaged: false },
    hasSettings: false,
    hasAliases: false,
    hasMappings: false } ] to sort of equal []
    at Assertion.assert (/dev/shm/workspace/kibana/packages/kbn-expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/kibana/packages/kbn-expect/expect.js:244:8)
    at Context.it (test/api_integration/apis/management/index_management/templates.js:44:43)
  actual:
   '[\n  {\n    "_kbnMeta": {\n      "isManaged": false\n    }\n    "composedOf": [\n      "metrics-mappings"\n      "metrics-settings"\n    ]\n    "hasAliases": false\n    "hasMappings": false\n    "hasSettings": false\n    "indexPatterns": [\n      "metrics-*-*"\n    ]\n    "name": "metrics"\n    "priority": 100\n    "version": 0\n  }\n  {\n    "_kbnMeta": {\n      "isManaged": false\n    }\n    "composedOf": [\n      "logs-mappings"\n      "logs-settings"\n    ]\n    "hasAliases": false\n    "hasMappings": false\n    "hasSettings": false\n    "indexPatterns": [\n      "logs-*-*"\n    ]\n    "name": "logs"\n    "priority": 100\n    "version": 0\n  }\n]',
  expected: '[]',
  showDiff: true }

Kibana Pipeline / kibana-xpack-agent / X-Pack API Integration Tests.x-pack/test/api_integration/apis/management/index_management/templates·js.apis management index management index templates get all should list all the index templates with the expected parameters

Link to Jenkins

Standard Out

Failed Tests Reporter:
  - Test has failed 1 times on tracked branches: https://dryrun

[00:00:00]       │
[00:00:00]         │ proc [kibana]   log   [15:52:25.901] [warning][plugins][reporting] Enabling the Chromium sandbox provides an additional layer of protection.
[00:00:00]         └-: apis
[00:00:00]           └-> "before all" hook
[00:04:47]           └-: management
[00:04:47]             └-> "before all" hook
[00:05:07]             └-: index management
[00:05:07]               └-> "before all" hook
[00:05:11]               └-: index templates
[00:05:11]                 └-> "before all" hook
[00:05:11]                 └-: get all
[00:05:11]                   └-> "before all" hook
[00:05:11]                   └-> should list all the index templates with the expected parameters
[00:05:11]                     └-> "before each" hook: global before each
[00:05:11]                     └-> "before each" hook
[00:05:11]                       │ info [o.e.c.m.MetadataIndexTemplateService] [kibana-ci-immutable-debian-tests-xl-1593702143659934750] adding template [template-fliwbsthsmgz-1593705145934] for index patterns [test*]
[00:05:11]                     └- ✖ fail: "apis management index management index templates get all should list all the index templates with the expected parameters"
[00:05:11]                     │

Stack Trace

{ Error: expected [ { name: 'metrics',
    version: 0,
    priority: 100,
    indexPatterns: [ 'metrics-*-*' ],
    composedOf: [ 'metrics-mappings', 'metrics-settings' ],
    _kbnMeta: { isManaged: false },
    hasSettings: false,
    hasAliases: false,
    hasMappings: false },
  { name: 'logs',
    version: 0,
    priority: 100,
    indexPatterns: [ 'logs-*-*' ],
    composedOf: [ 'logs-mappings', 'logs-settings' ],
    _kbnMeta: { isManaged: false },
    hasSettings: false,
    hasAliases: false,
    hasMappings: false } ] to sort of equal []
    at Assertion.assert (/dev/shm/workspace/kibana/packages/kbn-expect/expect.js:100:11)
    at Assertion.eql (/dev/shm/workspace/kibana/packages/kbn-expect/expect.js:244:8)
    at Context.it (test/api_integration/apis/management/index_management/templates.js:44:43)
  actual:
   '[\n  {\n    "_kbnMeta": {\n      "isManaged": false\n    }\n    "composedOf": [\n      "metrics-mappings"\n      "metrics-settings"\n    ]\n    "hasAliases": false\n    "hasMappings": false\n    "hasSettings": false\n    "indexPatterns": [\n      "metrics-*-*"\n    ]\n    "name": "metrics"\n    "priority": 100\n    "version": 0\n  }\n  {\n    "_kbnMeta": {\n      "isManaged": false\n    }\n    "composedOf": [\n      "logs-mappings"\n      "logs-settings"\n    ]\n    "hasAliases": false\n    "hasMappings": false\n    "hasSettings": false\n    "indexPatterns": [\n      "logs-*-*"\n    ]\n    "name": "logs"\n    "priority": 100\n    "version": 0\n  }\n]',
  expected: '[]',
  showDiff: true }

and 1 more failures, only showing the first 3.

Build metrics

✅ unchanged

History

💚 Build #57999 succeeded f5fcbf3
💔 Build #57972 failed 21953cc
💚 Build #57550 succeeded fd7046f
💔 Build #57265 failed 135b43a
💔 Build #57177 failed fb35a49

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

fix: discover and tsvb theming issues

2102674

nickofthyme requested review from cchaos and markov00 June 23, 2020 15:04

nickofthyme requested a review from a team as a code owner June 23, 2020 15:04

nickofthyme requested a review from a team June 23, 2020 15:04

nickofthyme added the bug Fixes for quality problems that affect the customer experience label Jun 23, 2020

fix: broken jest tests

18771dc

cchaos reviewed Jun 23, 2020

View reviewed changes

YulNaumenko self-requested a review June 23, 2020 21:47

nickofthyme added 2 commits June 23, 2020 17:39

test: fix charts theme tests based on changes

aff4e8a

fix: charts plugin theme readme spelling errors

189843c

YulNaumenko requested changes Jun 23, 2020

View reviewed changes

fix: broken hook refactoring

6207f94

YulNaumenko reviewed Jun 23, 2020

View reviewed changes

...ers_actions_ui/public/application/components/builtin_alert_types/threshold/visualization.tsx Show resolved Hide resolved

YulNaumenko approved these changes Jun 23, 2020

View reviewed changes

stratoula reviewed Jun 24, 2020

View reviewed changes

fix: tsvb border for bgColor with alpha values

d82c6b8

nickofthyme requested a review from a team as a code owner June 24, 2020 13:59

nickofthyme requested review from cchaos and stratoula June 24, 2020 14:39

cchaos reviewed Jun 24, 2020

View reviewed changes

cchaos approved these changes Jun 25, 2020

View reviewed changes

nickofthyme added 2 commits June 25, 2020 09:37

fix: add comment to style changes

730c2e9

Merge branch 'master' into fix-chart-bg-color

2c9cd0b

nickofthyme linked an issue Jun 29, 2020 that may be closed by this pull request

[Lens] XY and pie charts don't respect dark mode #70181

Closed

nickofthyme mentioned this pull request Jun 29, 2020

[Lens] XY and pie charts don't respect dark mode #70181

Closed

nickofthyme added 2 commits June 29, 2020 08:42

Merge branch 'master' into fix-chart-bg-color

cb726f3

fix chart bgColor issues, update lens to use charts plugin

fb35a49

nickofthyme requested a review from a team as a code owner June 29, 2020 14:58

fix dependency issue

135b43a

nickofthyme added 2 commits June 30, 2020 10:20

Update charts plugin parameters

faece28

- fix broken jest tests in lens - update datapannel to use charts theme - update FieldsAccordion to use charts theme

Merge branch 'master' into fix-chart-bg-color

fd7046f

jbudz approved these changes Jun 30, 2020

View reviewed changes

Merge branch 'master' into fix-chart-bg-color

21953cc

fix type error with XYChart

f5fcbf3

flash1293 reviewed Jul 2, 2020

View reviewed changes

nickofthyme merged commit f8ba824 into elastic:master Jul 2, 2020

nickofthyme deleted the fix-chart-bg-color branch July 2, 2020 14:02

nickofthyme mentioned this pull request Jul 2, 2020

[7.x] Fix discover, tsvb and Lens chart theming issues (#69695) #70598

Merged

nickofthyme changed the title ~~fix: discover and tsvb theming issues~~ Fix discover, tsvb and Lens chart theming issues Jul 2, 2020

nickofthyme added a commit that referenced this pull request Jul 2, 2020

Fix discover, tsvb and Lens chart theming issues (#69695) (#70598)

0ffa2cb

		@@ -0,0 +1,92 @@
		# Theme Service

		The `theme` service offers utilities to interact with the kibana theme. EUI provides a light and dark theme object to suplement the Elastic-Charts `baseTheme`. However, every instance of a Chart would need to pass down this the correctly EUI theme depending on Kibana's light or dark mode. There are several ways you can use the `theme` service to get the correct shared `theme` and `baseTheme`.


		## Why have `theme` and `baseTheme`?

		The `theme` prop is a recursive partial `Theme` that overrides properties from the `baseTheme`. This allows changes to the `Theme` type in `@elastic/charts` without having to update the `@elastic/eui` themes or every `<Chart />`.

	export function getTheme(darkMode: boolean, bgColor?: string \| null): Theme {
	if (!isValidColor(bgColor)) {
	return darkMode ? DARK_THEME : LIGHT_THEME;
	}

	const bgLuminosity = computeRelativeLuminosity(bgColor);
	const mainTheme = bgLuminosity <= 0.179 ? DARK_THEME : LIGHT_THEME;
	const color = findBestContrastColor(
	bgColor,
	LIGHT_THEME.axes.axisTitleStyle.fill,
	DARK_THEME.axes.axisTitleStyle.fill
	);
	return {
	...mainTheme,
	axes: {
	...mainTheme.axes,
	axisTitleStyle: {
	...mainTheme.axes.axisTitleStyle,
	fill: color,
	},
	tickLabelStyle: {
	...mainTheme.axes.tickLabelStyle,
	fill: color,
	},
	axisLineStyle: {
	...mainTheme.axes.axisLineStyle,
	stroke: color,
	},
	tickLineStyle: {
	...mainTheme.axes.tickLineStyle,
	stroke: color,
	},
	},
	};
	}

		@@ -26,72 +26,4 @@ Truncated color mappings in `value`/`text` form

Conversation

nickofthyme commented Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Screenshots

TSVB

Discover

Checklist

Uh oh!

elasticmachine commented Jun 23, 2020

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme Jun 23, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

YulNaumenko left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

YulNaumenko left a comment

Choose a reason for hiding this comment

Uh oh!

stratoula Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme Jun 24, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stratoula commented Jun 24, 2020

Uh oh!

nickofthyme commented Jun 24, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Jun 25, 2020

Uh oh!

cchaos left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nickofthyme commented Jun 25, 2020

nickofthyme commented Jun 23, 2020 •

edited

Loading

nickofthyme Jun 23, 2020 •

edited

Loading

stratoula Jun 24, 2020 •

edited

Loading

nickofthyme Jun 24, 2020 •

edited

Loading

nickofthyme commented Jun 29, 2020 •

edited

Loading