Skip to content

Commit

Permalink
feat: remove empty state from insight accordion (#137872)
Browse files Browse the repository at this point in the history
Since the UI can get quite cluttered with some insight accordions greyed out and some not, we decided (with the design team) to remove the dedicated empty space for now.
  • Loading branch information
janmonschke committed Aug 3, 2022
1 parent 40ce451 commit 041bd90
Show file tree
Hide file tree
Showing 7 changed files with 64 additions and 71 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,6 @@ describe('InsightAccordion', () => {
expect(screen.getByText(errorText)).toBeInTheDocument();
});

it("shows the text and a disabled button when it's in the empty state", () => {
const text = 'the text';
render(
<TestProviders>
<InsightAccordion state="empty" text={text} prefix="" renderContent={noopRenderer} />
</TestProviders>
);

const button = screen.getByRole('button', { name: text });
expect(button).toBeInTheDocument();
expect(button).toHaveAttribute('aria-disabled');
});

it('shows the text and renders the correct content', () => {
const text = 'the text';
const contentText = 'content text';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,7 @@ const StyledAccordion = euiStyled(EuiAccordion)`
border-radius: 6px;
`;

const EmptyAccordion = euiStyled(StyledAccordion)`
color: ${({ theme }) => theme.eui.euiColorDisabledText};
pointer-events: none;
`;

export type InsightAccordionState = 'loading' | 'error' | 'success' | 'empty';
export type InsightAccordionState = 'loading' | 'error' | 'success';

interface Props {
prefix: string;
Expand Down Expand Up @@ -61,24 +56,6 @@ export const InsightAccordion = React.memo<Props>(
onToggle={onToggle}
/>
);
case 'empty':
// Since EuiAccordions don't have an empty state and they don't allow to style the arrow
// we're using a custom styled Accordion here and we're adding the faded-out button manually.
return (
<EmptyAccordion
id={accordionId}
buttonContent={
<span>
<EuiIcon type="arrowRight" style={{ margin: '0px 8px 0 4px' }} />
{text}
</span>
}
buttonProps={{
'aria-disabled': true,
}}
arrowDisplay="none"
/>
);
case 'success':
// The accordion can display the content now
return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ import { InsightAccordion } from './insight_accordion';
import { SimpleAlertTable } from './simple_alert_table';
import { InvestigateInTimelineButton } from '../table/investigate_in_timeline_button';
import { ACTION_INVESTIGATE_IN_TIMELINE } from '../../../../detections/components/alerts_table/translations';
import { PROCESS_ANCESTRY, PROCESS_ANCESTRY_COUNT, PROCESS_ANCESTRY_ERROR } from './translations';
import {
PROCESS_ANCESTRY,
PROCESS_ANCESTRY_COUNT,
PROCESS_ANCESTRY_EMPTY,
PROCESS_ANCESTRY_ERROR,
} from './translations';

interface Props {
data: TimelineEventsDetailsItem;
Expand Down Expand Up @@ -65,12 +70,15 @@ export const RelatedAlertsByProcessAncestry = React.memo<Props>(
const [cache, setCache] = useState<Partial<Cache>>({});

const onToggle = useCallback((isOpen: boolean) => setShowContent(isOpen), []);
const isEmpty = !!cache.alertIds && cache.alertIds.length === 0;

// Makes sure the component is not fetching data before the accordion
// has been openend.
const renderContent = useCallback(() => {
if (!showContent) {
return null;
} else if (isEmpty) {
return PROCESS_ANCESTRY_EMPTY;
} else if (cache.alertIds) {
return (
<ActualRelatedAlertsByProcessAncestry
Expand All @@ -90,16 +98,14 @@ export const RelatedAlertsByProcessAncestry = React.memo<Props>(
onCacheLoad={setCache}
/>
);
}, [showContent, cache, data, eventId, timelineId, index, originalDocumentId]);

const isEmpty = !!cache.alertIds && cache.alertIds.length === 0;
}, [showContent, cache, data, eventId, timelineId, index, originalDocumentId, isEmpty]);

return (
<InsightAccordion
prefix="RelatedAlertsByProcessAncestry"
// `renderContent` and the associated sub-components are making sure to
// render the correct loading and error states so we can omit these states here
state={isEmpty ? 'empty' : 'success'}
state="success"
text={
// If we have fetched the alerts, display the count here, otherwise omit the count
cache.alertIds ? PROCESS_ANCESTRY_COUNT(cache.alertIds.length) : PROCESS_ANCESTRY
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import { InvestigateInTimelineButton } from '../table/investigate_in_timeline_bu
import { SimpleAlertTable } from './simple_alert_table';
import { getEnrichedFieldInfo } from '../helpers';
import { ACTION_INVESTIGATE_IN_TIMELINE } from '../../../../detections/components/alerts_table/translations';
import { SESSION_LOADING, SESSION_ERROR, SESSION_COUNT } from './translations';
import { SESSION_LOADING, SESSION_EMPTY, SESSION_ERROR, SESSION_COUNT } from './translations';

interface Props {
browserFields: BrowserFields;
Expand Down Expand Up @@ -64,9 +64,20 @@ export const RelatedAlertsBySession = React.memo<Props>(
fieldType: fieldFromBrowserField?.type,
});

const isEmpty = count === 0;

let state: InsightAccordionState = 'loading';
if (error) {
state = 'error';
} else if (alertIds || isEmpty) {
state = 'success';
}

const renderContent = useCallback(() => {
if (!alertIds || !cellData?.dataProviders) {
return null;
} else if (isEmpty && state !== 'loading') {
return SESSION_EMPTY;
}
return (
<>
Expand All @@ -80,16 +91,7 @@ export const RelatedAlertsBySession = React.memo<Props>(
</InvestigateInTimelineButton>
</>
);
}, [alertIds, cellData?.dataProviders]);

let state: InsightAccordionState = 'loading';
if (error) {
state = 'error';
} else if (count === 0) {
state = 'empty';
} else if (alertIds) {
state = 'success';
}
}, [alertIds, cellData?.dataProviders, isEmpty, state]);

return (
<InsightAccordion
Expand All @@ -111,7 +113,6 @@ function getTextFromState(state: InsightAccordionState, count: number | undefine
case 'error':
return SESSION_ERROR;
case 'success':
case 'empty':
return SESSION_COUNT(count);
default:
return '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ import { InvestigateInTimelineButton } from '../table/investigate_in_timeline_bu
import { SimpleAlertTable } from './simple_alert_table';
import { getEnrichedFieldInfo } from '../helpers';
import { ACTION_INVESTIGATE_IN_TIMELINE } from '../../../../detections/components/alerts_table/translations';
import { SOURCE_EVENT_LOADING, SOURCE_EVENT_ERROR, SOURCE_EVENT_COUNT } from './translations';
import {
SOURCE_EVENT_LOADING,
SOURCE_EVENT_EMPTY,
SOURCE_EVENT_ERROR,
SOURCE_EVENT_COUNT,
} from './translations';

interface Props {
browserFields: BrowserFields;
Expand Down Expand Up @@ -64,9 +69,20 @@ export const RelatedAlertsBySourceEvent = React.memo<Props>(
fieldType: fieldFromBrowserField?.type,
});

const isEmpty = count === 0;

let state: InsightAccordionState = 'loading';
if (error) {
state = 'error';
} else if (alertIds) {
state = 'success';
}

const renderContent = useCallback(() => {
if (!alertIds || !cellData?.dataProviders) {
return null;
} else if (isEmpty && state !== 'loading') {
return SOURCE_EVENT_EMPTY;
}
return (
<>
Expand All @@ -80,16 +96,7 @@ export const RelatedAlertsBySourceEvent = React.memo<Props>(
</InvestigateInTimelineButton>
</>
);
}, [alertIds, cellData?.dataProviders]);

let state: InsightAccordionState = 'loading';
if (error) {
state = 'error';
} else if (count === 0) {
state = 'empty';
} else if (alertIds) {
state = 'success';
}
}, [alertIds, cellData?.dataProviders, isEmpty, state]);

return (
<InsightAccordion
Expand All @@ -109,7 +116,6 @@ function getTextFromState(state: InsightAccordionState, count: number | undefine
case 'error':
return SOURCE_EVENT_ERROR;
case 'success':
case 'empty':
return SOURCE_EVENT_COUNT(count);
default:
return '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export const RelatedCases = React.memo<Props>(({ eventId }) => {
const toasts = useToasts();

const [relatedCases, setRelatedCases] = useState<RelatedCaseList | undefined>(undefined);
const [areCasesLoading, setAreCasesLoading] = useState(true);
const [hasError, setHasError] = useState<boolean>(false);

const renderContent = useCallback(() => renderCaseContent(relatedCases), [relatedCases]);
Expand All @@ -50,7 +49,6 @@ export const RelatedCases = React.memo<Props>(({ eventId }) => {
toasts.addWarning(CASES_ERROR_TOAST(error));
}
setRelatedCases(relatedCaseList);
setAreCasesLoading(false);
}, [eventId, cases.api, toasts]);

useEffect(() => {
Expand All @@ -60,8 +58,6 @@ export const RelatedCases = React.memo<Props>(({ eventId }) => {
let state: InsightAccordionState = 'loading';
if (hasError) {
state = 'error';
} else if (!areCasesLoading && relatedCases?.length === 0) {
state = 'empty';
} else if (relatedCases) {
state = 'success';
}
Expand Down Expand Up @@ -121,7 +117,6 @@ function getTextFromState(state: InsightAccordionState, caseCount = 0) {
case 'error':
return CASES_ERROR;
case 'success':
case 'empty':
return CASES_COUNT(caseCount);
default:
return '';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ export const PROCESS_ANCESTRY_ERROR = i18n.translate(
}
);

export const PROCESS_ANCESTRY_EMPTY = i18n.translate(
'xpack.securitySolution.alertDetails.overview.insights.related_alerts_by_process_ancestry_empty',
{
defaultMessage: 'There are no related alerts by process ancestry.',
}
);

export const SESSION_LOADING = i18n.translate(
'xpack.securitySolution.alertDetails.overview.insights.related_alerts_by_source_event_loading',
{ defaultMessage: 'Loading related alerts by source event' }
Expand All @@ -46,6 +53,13 @@ export const SESSION_ERROR = i18n.translate(
}
);

export const SESSION_EMPTY = i18n.translate(
'xpack.securitySolution.alertDetails.overview.insights.related_alerts_by_session_empty',
{
defaultMessage: 'There are no related alerts by session',
}
);

export const SESSION_COUNT = (count?: number) =>
i18n.translate(
'xpack.securitySolution.alertDetails.overview.insights.related_alerts_by_session_count',
Expand All @@ -66,6 +80,13 @@ export const SOURCE_EVENT_ERROR = i18n.translate(
}
);

export const SOURCE_EVENT_EMPTY = i18n.translate(
'xpack.securitySolution.alertDetails.overview.insights.related_alerts_by_source_event_empty',
{
defaultMessage: 'There are no related alerts by source event',
}
);

export const SOURCE_EVENT_COUNT = (count?: number) =>
i18n.translate(
'xpack.securitySolution.alertDetails.overview.insights_related_alerts_by_source_event_count',
Expand Down

0 comments on commit 041bd90

Please sign in to comment.