Skip to content

Commit

Permalink
[Security solution] [Timeline] Improve timeline title and move descri…
Browse files Browse the repository at this point in the history
…ption to notes tab (#106544)

* Improve timeline title and move description to the notes tab

Truncate the title only in the UI
When the user hover the title we display the full title
Truncate the title if it appears in a table
  • Loading branch information
machadoum authored Aug 17, 2021
1 parent 15494cd commit 6a5a215
Show file tree
Hide file tree
Showing 27 changed files with 354 additions and 95 deletions.
11 changes: 11 additions & 0 deletions x-pack/plugins/security_solution/common/types/timeline/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,17 @@ export enum TimelineTabs {
eql = 'eql',
}

/**
* Used for scrolling top inside a tab. Especially when swiching tabs.
*/
export interface ScrollToTopEvent {
/**
* Timestamp of the moment when the event happened.
* The timestamp might be necessary for the scenario where the event could happen multiple times.
*/
timestamp: number;
}

// eslint-disable-next-line @typescript-eslint/no-explicit-any
type EmptyObject = Record<any, never>;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
TIMELINE_FLYOUT_WRAPPER,
TIMELINE_PANEL,
TIMELINE_TAB_CONTENT_EQL,
TIMELINE_TAB_CONTENT_GRAPHS_NOTES,
} from '../../screens/timeline';
import { createTimelineTemplate } from '../../tasks/api_calls/timelines';

Expand Down Expand Up @@ -90,7 +91,9 @@ describe('Timelines', (): void => {

it('can be added notes', () => {
addNotesToTimeline(getTimeline().notes);
cy.get(NOTES_TEXT).should('have.text', getTimeline().notes);
cy.get(TIMELINE_TAB_CONTENT_GRAPHS_NOTES)
.find(NOTES_TEXT)
.should('have.text', getTimeline().notes);
});

it('should update timeline after adding eql', () => {
Expand Down
17 changes: 9 additions & 8 deletions x-pack/plugins/security_solution/cypress/tasks/timeline.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,15 +133,16 @@ export const goToQueryTab = () => {

export const addNotesToTimeline = (notes: string) => {
goToNotesTab().then(() => {
cy.get(NOTES_TEXT_AREA).type(notes);
cy.root()
.pipe(($el) => {
$el.find(ADD_NOTE_BUTTON).trigger('click');
return $el.find(NOTES_TAB_BUTTON).find('.euiBadge');
})
.should('have.text', '1');
cy.get(NOTES_TAB_BUTTON)
.find('.euiBadge__text')
.then(($el) => {
const notesCount = parseInt($el.text(), 10);

cy.get(NOTES_TEXT_AREA).type(notes);
cy.get(ADD_NOTE_BUTTON).trigger('click');
cy.get(`${NOTES_TAB_BUTTON} .euiBadge`).should('have.text', `${notesCount + 1}`);
});
});

goToQueryTab();
goToNotesTab();
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,9 @@
*/

import { EuiButtonEmpty } from '@elastic/eui';
import React, { useRef, useState, useEffect, useCallback, ReactNode } from 'react';
import React, { useState, useCallback, ReactNode } from 'react';
import styled from 'styled-components';
import { useIsOverflow } from '../../hooks/use_is_overflow';
import * as i18n from './translations';

const LINE_CLAMP = 3;
Expand Down Expand Up @@ -39,29 +40,13 @@ const LineClampComponent: React.FC<{
children: ReactNode;
lineClampHeight?: number;
}> = ({ children, lineClampHeight = LINE_CLAMP_HEIGHT }) => {
const [isOverflow, setIsOverflow] = useState<boolean | null>(null);
const [isExpanded, setIsExpanded] = useState<boolean | null>(null);
const descriptionRef = useRef<HTMLDivElement>(null);
const [isOverflow, descriptionRef] = useIsOverflow(children);

const toggleReadMore = useCallback(() => {
setIsExpanded((prevState) => !prevState);
}, []);

useEffect(() => {
if (descriptionRef?.current?.clientHeight != null) {
if (
(descriptionRef?.current?.scrollHeight ?? 0) > (descriptionRef?.current?.clientHeight ?? 0)
) {
setIsOverflow(true);
}

if (
(descriptionRef?.current?.scrollHeight ?? 0) <= (descriptionRef?.current?.clientHeight ?? 0)
) {
setIsOverflow(false);
}
}
}, []);

if (isExpanded) {
return (
<>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ interface MarkdownEditorProps {
editorId?: string;
dataTestSubj?: string;
height?: number;
autoFocusDisabled?: boolean;
}

const MarkdownEditorComponent: React.FC<MarkdownEditorProps> = ({
Expand All @@ -26,16 +27,18 @@ const MarkdownEditorComponent: React.FC<MarkdownEditorProps> = ({
editorId,
dataTestSubj,
height,
autoFocusDisabled = false,
}) => {
const [markdownErrorMessages, setMarkdownErrorMessages] = useState([]);
const onParse = useCallback((err, { messages }) => {
setMarkdownErrorMessages(err ? [err] : messages);
}, []);

useEffect(
() => document.querySelector<HTMLElement>('textarea.euiMarkdownEditorTextArea')?.focus(),
[]
);
useEffect(() => {
if (!autoFocusDisabled) {
document.querySelector<HTMLElement>('textarea.euiMarkdownEditorTextArea')?.focus();
}
}, [autoFocusDisabled]);

return (
<EuiMarkdownEditor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@ describe('Scroll to top', () => {
Object.defineProperty(globalNode.window, 'scroll', { value: null });
Object.defineProperty(globalNode.window, 'scrollTo', { value: spyScrollTo });
mount(<HookWrapper hook={() => useScrollToTop()} />);

expect(spyScrollTo).toHaveBeenCalled();
});

test('should not scroll when `shouldScroll` is false', () => {
Object.defineProperty(globalNode.window, 'scroll', { value: spyScroll });
mount(<HookWrapper hook={() => useScrollToTop(undefined, false)} />);

expect(spyScrollTo).not.toHaveBeenCalled();
});

test('should scroll the element matching the given selector', () => {
const fakeElement = { scroll: spyScroll };
Object.defineProperty(globalNode.document, 'querySelector', {
value: () => fakeElement,
});
mount(<HookWrapper hook={() => useScrollToTop('fake selector')} />);

expect(spyScroll).toHaveBeenCalledWith(0, 0);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,22 @@

import { useEffect } from 'react';

export const useScrollToTop = () => {
/**
* containerSelector: The element with scrolling. It defaults to the window.
* shouldScroll: It should be used for conditional scrolling.
*/
export const useScrollToTop = (containerSelector?: string, shouldScroll = true) => {
useEffect(() => {
const container = containerSelector ? document.querySelector(containerSelector) : window;

if (!shouldScroll || !container) return;

// trying to use new API - https://developer.mozilla.org/en-US/docs/Web/API/Window/scrollTo
if (window.scroll) {
window.scroll(0, 0);
if (container.scroll) {
container.scroll(0, 0);
} else {
// just a fallback for older browsers
window.scrollTo(0, 0);
container.scrollTo(0, 0);
}
});

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/*
* 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.
*/

import { useEffect, useRef, useState } from 'react';

/**
* It checks if the element that receives the returned Ref has oveflow the max height.
*/
export const useIsOverflow: (
dependency: unknown
) => [isOveflow: boolean | null, ref: React.RefObject<HTMLDivElement>] = (dependency) => {
const [isOverflow, setIsOverflow] = useState<boolean | null>(null);
const ref = useRef<HTMLDivElement>(null);

useEffect(() => {
if (ref.current?.clientHeight != null) {
if ((ref?.current?.scrollHeight ?? 0) > (ref?.current?.clientHeight ?? 0)) {
setIsOverflow(true);
}

if ((ref.current?.scrollHeight ?? 0) <= (ref?.current?.clientHeight ?? 0)) {
setIsOverflow(false);
}
}
}, [ref, dependency]);

return [isOverflow, ref];
};
2 changes: 2 additions & 0 deletions x-pack/plugins/security_solution/public/common/mock/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import { defaultHeaders } from '../../timelines/components/timeline/body/column_
interface Global extends NodeJS.Global {
// eslint-disable-next-line @typescript-eslint/no-explicit-any
window?: any;
// eslint-disable-next-line @typescript-eslint/no-explicit-any
document?: any;
}

export const globalNode: Global = global;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,12 @@ const StyledEuiButtonEmpty = styled(EuiButtonEmpty)`
}
`;

const TitleConatiner = styled(EuiFlexItem)`
overflow: hidden;
display: inline-block;
text-overflow: ellipsis;
`;

const ActiveTimelinesComponent: React.FC<ActiveTimelinesProps> = ({
timelineId,
timelineStatus,
Expand Down Expand Up @@ -100,7 +106,7 @@ const ActiveTimelinesComponent: React.FC<ActiveTimelinesProps> = ({
/>
</EuiToolTip>
</EuiFlexItem>
<EuiFlexItem grow={false}>{title}</EuiFlexItem>
<TitleConatiner grow={false}>{title}</TitleConatiner>
{!isOpen && (
<EuiFlexItem grow={false}>
<TimelineEventsCountBadge />
Expand Down
Loading

0 comments on commit 6a5a215

Please sign in to comment.