Skip to content

Commit

Permalink
[EUI][APM] Update Hardcoded Colors (#203348)
Browse files Browse the repository at this point in the history
## Summary

This PR replaces a couple of places where hardcoded colors are used in
the APM portion of Kibana with EUITheme colors.
Before & After screenshots can be seen in the associated issue, #200960.
However, I was unable to find an example for the
[.../alert_details_app_section/failed_transaction_chart.tsx](https://github.com/elastic/kibana/pull/203348/files#diff-9d9e4bbfe128f4d2f6ff7f027cf746d679a6c06805ef77240cceb2770a837a28).
It seems like this chart in the alert creation flyout will never render
with annotations.

### Checklist

Check the PR satisfies following conditions. 
Reviewers should verify this PR satisfies this list as well.

- [x] The PR description includes the appropriate Release Notes section,
and the correct `release_note:*` label is applied per the
[guidelines](https://www.elastic.co/guide/en/kibana/master/contributing.html#kibana-release-notes-process)

---------

Co-authored-by: Miriam <[email protected]>
  • Loading branch information
bryce-b and MiriamAparicio authored Dec 12, 2024
1 parent b3e2b4b commit 278889a
Show file tree
Hide file tree
Showing 15 changed files with 107 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ import {
export const LABELS = 'labels'; // implies labels.* wildcard

export const APM_SERVICE_GROUP_SAVED_OBJECT_TYPE = 'apm-service-group';
export const SERVICE_GROUP_COLOR_DEFAULT = '#D1DAE7';
export const MAX_NUMBER_OF_SERVICE_GROUPS = 500;

export interface ServiceGroup {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
import { SettingsSpec } from '@elastic/charts';

export const DEFAULT_DATE_FORMAT = 'HH:mm:ss';
export const CHART_ANNOTATION_RED_COLOR = '#BD271E';
export const CHART_SETTINGS: Partial<SettingsSpec> = {
showLegend: false,
};
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
/* Error Rate */

import React from 'react';
import chroma from 'chroma-js';
import {
EuiFlexItem,
EuiPanel,
Expand Down Expand Up @@ -155,7 +154,7 @@ function FailedTransactionChart({
<AlertActiveTimeRangeAnnotation
alertStart={alertStart}
alertEnd={alertEnd}
color={chroma(transparentize('#F04E981A', 0.2)).hex().toUpperCase()}
color={transparentize(euiTheme.colors.danger, 0.2)}
id={'alertActiveRect'}
key={'alertActiveRect'}
/>,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import type { Filter } from '@kbn/es-query';
import { ApmPluginStartDeps } from '../../../../../plugin';
import { getLayerList } from './map_layers/get_layer_list';
import { MapTypes } from '../../../../../../common/mobile/constants';
import { StyleColorParams } from './map_layers/style_color_params';

function EmbeddedMapComponent({
selectedMap,
Expand All @@ -21,13 +22,19 @@ function EmbeddedMapComponent({
kuery = '',
filters,
dataView,
styleColors = {
lineColor: '',
labelColor: '',
labelOutlineColor: '',
},
}: {
selectedMap: MapTypes;
start: string;
end: string;
kuery?: string;
filters: Filter[];
dataView?: DataView;
styleColors?: StyleColorParams;
}) {
const { maps } = useKibana<ApmPluginStartDeps>().services;

Expand All @@ -37,9 +44,10 @@ function EmbeddedMapComponent({
selectedMap,
maps,
dataViewId: dataView?.id,
styleColors,
})
: [];
}, [selectedMap, maps, dataView?.id]);
}, [selectedMap, maps, dataView?.id, styleColors]);

return (
<div
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ import React, { useState } from 'react';
import { DataView } from '@kbn/data-views-plugin/common';
import { EuiSpacer } from '@elastic/eui';
import type { Filter } from '@kbn/es-query';
import { useEuiTheme } from '@elastic/eui';
import { EmbeddedMap } from './embedded_map';
import { MapTypes } from '../../../../../../common/mobile/constants';
import { EmbeddedMapSelect } from './embedded_map_select';
import { StyleColorParams } from './map_layers/style_color_params';

export function GeoMap({
start,
Expand All @@ -27,7 +29,13 @@ export function GeoMap({
dataView?: DataView;
}) {
const [selectedMap, selectMap] = useState(MapTypes.Http);
const { euiTheme } = useEuiTheme();

const styleColors = {
lineColor: euiTheme.colors.textParagraph,
labelColor: euiTheme.colors.textParagraph,
labelOutlineColor: euiTheme.colors.backgroundBasePlain,
} as StyleColorParams;
return (
<>
<EmbeddedMapSelect selectedMap={selectedMap} onChange={selectMap} />
Expand All @@ -44,6 +52,7 @@ export function GeoMap({
kuery={kuery}
filters={filters}
dataView={dataView}
styleColors={styleColors}
/>
</div>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
} from '../../../../../../../common/es_fields/apm';
import { getLayerStyle, PalleteColors } from './get_map_layer_style';
import { MobileSpanSubtype, MobileSpanType } from '../../../../../../../common/mobile/constants';
import { StyleColorParams } from './style_color_params';

interface VectorLayerDescriptor extends BaseVectorLayerDescriptor {
sourceDescriptor: EMSFileSourceDescriptor;
Expand All @@ -41,7 +42,11 @@ const label = i18n.translate('xpack.apm.serviceOverview.embeddedMap.httpRequests
defaultMessage: 'HTTP requests',
});

export function getHttpRequestsLayerList(maps: MapsStartApi | undefined, dataViewId: string) {
export function getHttpRequestsLayerList(
maps: MapsStartApi | undefined,
dataViewId: string,
styleColors: StyleColorParams
) {
const whereQuery = {
language: 'kuery',
query: `${PROCESSOR_EVENT}:${ProcessorEvent.span} and ${SPAN_SUBTYPE}:${MobileSpanSubtype.Http} and ${SPAN_TYPE}:${MobileSpanType.External}`,
Expand Down Expand Up @@ -74,7 +79,7 @@ export function getHttpRequestsLayerList(maps: MapsStartApi | undefined, dataVie
id: 'world_countries',
tooltipProperties: [COUNTRY_NAME],
},
style: getLayerStyle(HTTP_REQUEST_PER_COUNTRY, PalleteColors.BluetoRed),
style: getLayerStyle(HTTP_REQUEST_PER_COUNTRY, PalleteColors.BluetoRed, styleColors),
id: uuidv4(),
label: i18n.translate('xpack.apm.serviceOverview.embeddedMap.httpRequests.country.label', {
defaultMessage: 'HTTP requests per country',
Expand Down Expand Up @@ -113,7 +118,7 @@ export function getHttpRequestsLayerList(maps: MapsStartApi | undefined, dataVie
id: 'administrative_regions_lvl2',
tooltipProperties: ['region_iso_code'],
},
style: getLayerStyle(HTTP_REQUESTS_PER_REGION, PalleteColors.YellowtoRed),
style: getLayerStyle(HTTP_REQUESTS_PER_REGION, PalleteColors.YellowtoRed, styleColors),
id: uuidv4(),
label: i18n.translate('xpack.apm.serviceOverview.embeddedMap.httpRequests.region.label', {
defaultMessage: 'HTTP requests per region',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,23 +8,26 @@ import type { MapsStartApi } from '@kbn/maps-plugin/public';
import { LayerDescriptor } from '@kbn/maps-plugin/common';
import { getHttpRequestsLayerList } from './get_http_requests_map_layer_list';
import { getSessionMapLayerList } from './get_session_map_layer_list';
import { StyleColorParams } from './style_color_params';
import { MapTypes } from '../../../../../../../common/mobile/constants';

export function getLayerList({
selectedMap,
maps,
dataViewId,
styleColors,
}: {
selectedMap: MapTypes;
maps: MapsStartApi | undefined;
dataViewId: string;
styleColors: StyleColorParams;
}): LayerDescriptor[] {
switch (selectedMap) {
case MapTypes.Http:
return getHttpRequestsLayerList(maps, dataViewId);
return getHttpRequestsLayerList(maps, dataViewId, styleColors);
case MapTypes.Session:
return getSessionMapLayerList(maps, dataViewId);
return getSessionMapLayerList(maps, dataViewId, styleColors);
default:
return getHttpRequestsLayerList(maps, dataViewId);
return getHttpRequestsLayerList(maps, dataViewId, styleColors);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,19 @@ import {
SYMBOLIZE_AS_TYPES,
} from '@kbn/maps-plugin/common';

import { StyleColorParams } from './style_color_params';

export enum PalleteColors {
BluetoRed = 'Blue to Red',
YellowtoRed = 'Yellow to Red',
}

export function getLayerStyle(fieldName: string, color: PalleteColors): VectorStyleDescriptor {
export function getLayerStyle(
fieldName: string,
color: PalleteColors,
styleColors: StyleColorParams
): VectorStyleDescriptor {
const { lineColor, labelColor, labelOutlineColor } = styleColors;
return {
type: 'VECTOR',
properties: {
Expand All @@ -41,7 +48,7 @@ export function getLayerStyle(fieldName: string, color: PalleteColors): VectorSt
},
lineColor: {
type: STYLE_TYPE.DYNAMIC,
options: { color: '#3d3d3d', fieldMetaOptions: { isEnabled: true } },
options: { color: lineColor, fieldMetaOptions: { isEnabled: true } },
},
lineWidth: { type: STYLE_TYPE.STATIC, options: { size: 1 } },
iconSize: { type: STYLE_TYPE.STATIC, options: { size: 6 } },
Expand Down Expand Up @@ -72,12 +79,12 @@ export function getLayerStyle(fieldName: string, color: PalleteColors): VectorSt
},
labelColor: {
type: STYLE_TYPE.STATIC,
options: { color: '#3d3d3d' },
options: { color: labelColor },
},
labelSize: { type: STYLE_TYPE.STATIC, options: { size: 14 } },
labelBorderColor: {
type: STYLE_TYPE.STATIC,
options: { color: '#FFFFFF' },
options: { color: labelOutlineColor },
},
symbolizeAs: { options: { value: SYMBOLIZE_AS_TYPES.CIRCLE } },
labelBorderSize: { options: { size: LABEL_BORDER_SIZES.SMALL } },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
SESSION_ID,
} from '../../../../../../../common/es_fields/apm';
import { getLayerStyle, PalleteColors } from './get_map_layer_style';
import { StyleColorParams } from './style_color_params';

interface VectorLayerDescriptor extends BaseVectorLayerDescriptor {
sourceDescriptor: EMSFileSourceDescriptor;
Expand All @@ -36,7 +37,11 @@ const SESSION_PER_REGION = `__kbnjoin__cardinality_of_session.id__${PER_REGION_L
const label = i18n.translate('xpack.apm.serviceOverview.embeddedMap.session.metric.label', {
defaultMessage: 'Sessions',
});
export function getSessionMapLayerList(maps: MapsStartApi | undefined, dataViewId: string) {
export function getSessionMapLayerList(
maps: MapsStartApi | undefined,
dataViewId: string,
styleColors: StyleColorParams
) {
const sessionsByCountryLayer: VectorLayerDescriptor = {
joins: [
{
Expand Down Expand Up @@ -64,7 +69,7 @@ export function getSessionMapLayerList(maps: MapsStartApi | undefined, dataViewI
id: 'world_countries',
tooltipProperties: [COUNTRY_NAME],
},
style: getLayerStyle(SESSION_PER_COUNTRY, PalleteColors.BluetoRed),
style: getLayerStyle(SESSION_PER_COUNTRY, PalleteColors.BluetoRed, styleColors),
id: uuidv4(),
label: i18n.translate('xpack.apm.serviceOverview.embeddedMap.sessionCountry.metric.label', {
defaultMessage: 'Sessions per country',
Expand Down Expand Up @@ -103,7 +108,7 @@ export function getSessionMapLayerList(maps: MapsStartApi | undefined, dataViewI
id: 'administrative_regions_lvl2',
tooltipProperties: ['region_iso_code'],
},
style: getLayerStyle(SESSION_PER_REGION, PalleteColors.YellowtoRed),
style: getLayerStyle(SESSION_PER_REGION, PalleteColors.YellowtoRed, styleColors),
id: uuidv4(),
label: i18n.translate('xpack.apm.serviceOverview.embeddedMap.sessionRegion.metric.label', {
defaultMessage: 'Sessions per region',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/*
* 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; you may not use this file except in compliance with the Elastic License
* 2.0.
*/

export interface StyleColorParams {
lineColor: string;
labelColor: string;
labelOutlineColor: string;
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
useColorPickerState,
EuiText,
isValidHex,
useEuiTheme,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React, { useEffect, useState } from 'react';
Expand All @@ -43,7 +44,8 @@ export function GroupDetails({
isLoading,
titleId,
}: Props) {
const initialColor = serviceGroup?.color || '#5094C4';
const { euiTheme } = useEuiTheme();
const initialColor = serviceGroup?.color || euiTheme.colors.backgroundFilledPrimary;
const [name, setName] = useState(serviceGroup?.groupName);
const [color, setColor, colorPickerErrors] = useColorPickerState(initialColor);
const [description, setDescription] = useState<string | undefined>(serviceGroup?.description);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,11 @@ import {
EuiFlexItem,
EuiText,
useIsWithinBreakpoints,
useEuiTheme,
} from '@elastic/eui';
import { i18n } from '@kbn/i18n';
import React from 'react';
import { ServiceGroup, SERVICE_GROUP_COLOR_DEFAULT } from '../../../../../common/service_groups';
import { ServiceGroup } from '../../../../../common/service_groups';
import { useObservabilityActiveAlertsHref } from '../../../shared/links/kibana';
import { ServiceStat } from './service_stat';

Expand All @@ -30,15 +31,15 @@ interface Props {

export function ServiceGroupsCard({ serviceGroup, href, serviceGroupCounts, isLoading }: Props) {
const isMobile = useIsWithinBreakpoints(['xs', 's']);

const { euiTheme } = useEuiTheme();
const activeAlertsHref = useObservabilityActiveAlertsHref(serviceGroup.kuery);
const cardProps: EuiCardProps = {
style: { width: isMobile ? '100%' : 286 },
icon: (
<>
<EuiAvatar
name={serviceGroup.groupName}
color={serviceGroup.color || SERVICE_GROUP_COLOR_DEFAULT}
color={serviceGroup.color || euiTheme.colors.backgroundFilledPrimary}
size="l"
/>
</>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,52 +8,58 @@
import React from 'react';
import { mount } from 'enzyme';
import { HttpStatusBadge } from '.';
import {
successColor,
neutralColor,
warningColor,
errorColor,
} from '../../../../utils/http_status_code_to_color';
import { renderHook } from '@testing-library/react-hooks';
import { useEuiTheme } from '@elastic/eui';

describe('HttpStatusBadge', () => {
describe('render', () => {
describe('with status code 100', () => {
it('renders with neutral color', () => {
const wrapper = mount(<HttpStatusBadge status={100} />);

expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(neutralColor);
const { result } = renderHook(() => useEuiTheme());
expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(
result.current.euiTheme.colors.vis.euiColorVisGrey0
);
});
});

describe('with status code 200', () => {
it('renders with success color', () => {
const wrapper = mount(<HttpStatusBadge status={200} />);

expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(successColor);
const { result } = renderHook(() => useEuiTheme());
expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(
result.current.euiTheme.colors.vis.euiColorVisSuccess0
);
});
});

describe('with status code 301', () => {
it('renders with neutral color', () => {
const wrapper = mount(<HttpStatusBadge status={301} />);

expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(neutralColor);
const { result } = renderHook(() => useEuiTheme());
expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(
result.current.euiTheme.colors.vis.euiColorVisGrey0
);
});
});

describe('with status code 404', () => {
it('renders with warning color', () => {
const wrapper = mount(<HttpStatusBadge status={404} />);

expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(warningColor);
const { result } = renderHook(() => useEuiTheme());
expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(
result.current.euiTheme.colors.vis.euiColorVisWarning0
);
});
});

describe('with status code 502', () => {
it('renders with error color', () => {
const wrapper = mount(<HttpStatusBadge status={502} />);

expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(errorColor);
const { result } = renderHook(() => useEuiTheme());
expect(wrapper.find('HttpStatusBadge EuiBadge').prop('color')).toEqual(
result.current.euiTheme.colors.vis.euiColorVisDanger0
);
});
});

Expand Down
Loading

0 comments on commit 278889a

Please sign in to comment.