Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Address couples of feedback. Issue: #26, #36, #37, #38 #45

Merged
merged 5 commits into from
Apr 29, 2020
Merged

Address couples of feedback. Issue: #26, #36, #37, #38 #45

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
914b5b1
Address couples of feedback. Issue: #26, #36, #37, #38
yizheliu-amazon Apr 24, 2020
199f1be
Minor fix
yizheliu-amazon Apr 24, 2020
a79940f
Increase display time range of live anomaly chart to display data poi…
yizheliu-amazon Apr 25, 2020
ef7719d
changed style for ContentPanel and LiveChart/DistributionChart, and a…
yizheliu-amazon Apr 28, 2020
beab5a4
Address Yaliang's comment, and change height of live chart to 400px i…
yizheliu-amazon Apr 28, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions public/components/ContentPanel/ContentPanel.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@
*/

import React from 'react';
//@ts-ignore

import {
//@ts-ignore
EuiTitleSize,
EuiFlexGroup,
EuiFlexItem,
Expand All @@ -38,22 +39,27 @@ type ContentPanelProps = {
titleContainerStyles?: React.CSSProperties;
actions?: React.ReactNode | React.ReactNode[];
children: React.ReactNode | React.ReactNode[];
contentPanelClassName?: string;
};

const ContentPanel = (props: ContentPanelProps) => (
<EuiPanel
style={{ paddingLeft: '0px', paddingRight: '0px', ...props.panelStyles }}
style={{
padding: '20px 0px',
...props.panelStyles,
}}
className={props.contentPanelClassName}
>
<EuiFlexGroup
style={{ padding: '0px 10px', ...props.titleContainerStyles }}
style={{ padding: '0px 20px', ...props.titleContainerStyles }}
Copy link
Contributor

Choose a reason for hiding this comment

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

How about we change this padding as 0

style={{ padding: '0px', ...props.titleContainerStyles }}

And set the padding of EuiPanel as 20px, so we can align title and content body.

<EuiPanel
    style={{ padding: '20px', ...props.panelStyles }}
    className={props.contentPanelClassName}
  >

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changing the padding of EuiPanel to 20px will make the EuiHorizontalRule looks disconnected to the Panel. I changed the style for Live chart, and below is the screenshot.
Screen Shot 2020-04-27 at 5 52 28 PM

This is not same as the UX mock up.

Copy link
Contributor

Choose a reason for hiding this comment

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

Sure, let's follow the UX mock up

justifyContent="spaceBetween"
alignItems="center"
>
<EuiFlexItem>
{typeof props.title === 'string' ? (
<EuiTitle
size={props.titleSize || 'l'}
className={props.titleClassName || 'content-panel-title'}
className={props.titleClassName || ''}
>
<h3>{props.title}</h3>
</EuiTitle>
Expand Down Expand Up @@ -105,7 +111,14 @@ const ContentPanel = (props: ContentPanelProps) => (
className={props.horizontalRuleClassName}
/>
)}
<div style={{ padding: '0px 10px', ...props.bodyStyles }}>
<div
style={{
paddingTop: '12px',
paddingLeft: '20px',
paddingRight: '20px',
...props.bodyStyles,
}}
>
{props.children}
</div>
</EuiPanel>
Expand Down
2 changes: 1 addition & 1 deletion public/components/ContentPanel/__snapshots__/ContentPanel.test.tsx.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ exports[`<ContentPanel /> spec renders the component 1`] = `
class="euiFlexItem"
>
<h3
class="euiTitle euiTitle--large content-panel-title"
class="euiTitle euiTitle--large"
>
Testing
</h3>
Expand Down
5 changes: 0 additions & 5 deletions public/components/ContentPanel/index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,8 @@
* permissions and limitations under the License.
*/

.content-panel-title {
color: #3f3f3f;
}

.content-panel-subTitle {
color: #879196;
font-family: 'Helvetica Neue';
font-size: 12px;
letter-spacing: 0;
}
145 changes: 73 additions & 72 deletions public/pages/Dashboard/Components/AnomaliesDistribution.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,9 @@ import { useDispatch } from 'react-redux';
import { Datum } from '@elastic/charts/dist/utils/commons';
import React from 'react';
import { TIME_RANGE_OPTIONS } from '../../Dashboard/utils/constants';
import { get } from 'lodash';
import { get, isEmpty } from 'lodash';
import { searchES } from '../../../redux/reducers/elasticsearch';
import { MAX_DETECTORS } from '../../../utils/constants';
import { MAX_DETECTORS, MAX_ANOMALIES } from '../../../utils/constants';
import { AD_DOC_FIELDS } from '../../../../server/utils/constants';
export interface AnomaliesDistributionChartProps {
allDetectorsSelected: boolean;
Expand Down Expand Up @@ -64,17 +64,23 @@ export const AnomaliesDistributionChart = (
const [timeRange, setTimeRange] = useState(TIME_RANGE_OPTIONS[0].value);

const getAnomalyResult = async (currentDetectors: DetectorListItem[]) => {
const finalAnomalyResult = await getLatestAnomalyResultsForDetectorsByTimeRange(
const latestAnomalyResult = await getLatestAnomalyResultsForDetectorsByTimeRange(
searchES,
props.selectedDetectors,
timeRange,
MAX_DETECTORS,
dispatch
dispatch,
0,
MAX_ANOMALIES,
MAX_DETECTORS
);
setAnomalyResults(finalAnomalyResult);

const nonZeroAnomalyResult = latestAnomalyResult.filter(
anomalyData => get(anomalyData, AD_DOC_FIELDS.ANOMALY_GRADE, 0) > 0
);
setAnomalyResults(nonZeroAnomalyResult);

const resultDetectors = getFinalDetectors(
finalAnomalyResult,
nonZeroAnomalyResult,
props.selectedDetectors
);
setIndicesNumber(getFinalIndices(resultDetectors).size);
Expand Down Expand Up @@ -123,9 +129,8 @@ export const AnomaliesDistributionChart = (
<EuiFlexItem>
<EuiText className={'anomaly-distribution-subtitle'}>
<p>
{
'The inner circle shows the anomaly distribution by your indices. The outer circle shows the anomaly distribution by your detector'
}
{'The inner circle shows the anomaly distribution by your indices. ' +
'The outer circle shows the anomaly distribution by your detectors.'}
</p>
</EuiText>
</EuiFlexItem>
Expand All @@ -141,7 +146,7 @@ export const AnomaliesDistributionChart = (
/>
}
>
<EuiFlexGroup style={{ padding: '10px' }}>
<EuiFlexGroup>
<EuiFlexItem>
<EuiStat
description={'Indices with anomalies'}
Expand All @@ -161,80 +166,76 @@ export const AnomaliesDistributionChart = (
</EuiFlexGroup>
{anomalyResultsLoading ? (
<EuiFlexGroup justifyContent="center">
<EuiFlexItem grow={false}>
<EuiLoadingChart size="m" />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiLoadingChart size="l" />
</EuiFlexItem>
<EuiFlexItem grow={false}>
<EuiLoadingChart size="xl" />
</EuiFlexItem>
</EuiFlexGroup>
) : (
<EuiFlexGroup justifyContent="center">
<EuiFlexItem grow={false}>
<Chart className="anomalies-distribution-sunburst">
<Partition
id="Anomalies by index and detector"
data={visualizeAnomalyResultForSunburstChart(
anomalyResults,
finalDetectors
)}
valueAccessor={(d: Datum) => d.count as number}
valueFormatter={(d: number) => d.toString()}
layers={[
{
groupByRollup: (d: Datum) => d.indices,
nodeLabel: (d: Datum) => {
return d;
},
fillLabel: {
textInvertible: true,
},
shape: {
fillColor: d => {
return fillOutColors(
d,
(d.x0 + d.x1) / 2 / (2 * Math.PI),
[]
);
{isEmpty(anomalyResults) ? null : (
<Chart className="anomalies-distribution-sunburst">
<Partition
id="Anomalies by index and detector"
data={visualizeAnomalyResultForSunburstChart(
anomalyResults,
finalDetectors
)}
valueAccessor={(d: Datum) => d.count as number}
valueFormatter={(d: number) => d.toString()}
layers={[
{
groupByRollup: (d: Datum) => d.indices,
nodeLabel: (d: Datum) => {
return d;
},
fillLabel: {
textInvertible: true,
},
shape: {
fillColor: d => {
return fillOutColors(
d,
(d.x0 + d.x1) / 2 / (2 * Math.PI),
[]
);
},
},
},
},
{
groupByRollup: (d: Datum) => d.name,
nodeLabel: (d: Datum) => {
return d;
{
groupByRollup: (d: Datum) => d.name,
nodeLabel: (d: Datum) => {
return d;
},
fillLabel: {
textInvertible: true,
},
shape: {
fillColor: d => {
return fillOutColors(
d,
(d.x0 + d.x1) / 2 / (2 * Math.PI),
[]
);
},
},
},
]}
config={{
partitionLayout: PartitionLayout.sunburst,
fontFamily: 'Arial',
outerSizeRatio: 1,
fillLabel: {
textInvertible: true,
},
shape: {
fillColor: d => {
return fillOutColors(
d,
(d.x0 + d.x1) / 2 / (2 * Math.PI),
[]
);
},
},
},
]}
config={{
partitionLayout: PartitionLayout.sunburst,
fontFamily: 'Arial',
outerSizeRatio: 1,
fillLabel: {
textInvertible: true,
},
// TODO: Given only 1 detector exists, the inside Index circle will have issue in following scenarios:
// 1: if Linked Label is configured for identifying index, label of Index circle will be invisible;
// 2: if Fill Label is configured for identifying index, label of it will be overlapped with outer Detector circle
// Issue link: https://github.com/opendistro-for-elasticsearch/anomaly-detection-kibana-plugin/issues/24
}}
/>
</Chart>
// TODO: Given only 1 detector exists, the inside Index circle will have issue in following scenarios:
// 1: if Linked Label is configured for identifying index, label of Index circle will be invisible;
// 2: if Fill Label is configured for identifying index, label of it will be overlapped with outer Detector circle
// Issue link: https://github.com/opendistro-for-elasticsearch/anomaly-detection-kibana-plugin/issues/24
}}
/>
</Chart>
)}
</EuiFlexItem>
</EuiFlexGroup>
)}
Expand Down
Loading