Skip to content

Commit dca4a23

Browse files
andrew-goldsteinandrew-goldstein
authored
[Security Solution] Full screen fixes for Timeline based views (#73421)
## Full screen fixes for Timeline based views - Fixes an issue where sometimes, Global navigation is hidden until the page is scrolled when exiting full screen mode - Improves performance by adding an intent delay before showing the draggable wrapper hover menu - Removes an unnecessary CSS transition ### Sometimes, Global navigation is hidden until the page is scrolled when exiting full screen mode Sometimes, after exiting `Full screen` mode in a page, for example, the `Detections` page, the global navigation, e.g. `Overview Detections Hosts...` is hidden until the page is scrolled. To reproduce: 1) Navigate to the `Detections` page 2) Click the `Full screen` button in the table 3) Without scrolling the full screen view, click the `Exit full screen` button **Expected result** - [x] The global navigation e.g. `Overview Detections Hosts...` is visible above the search bar, per the screenshot below: ![correct-global-navigation](https://user-images.githubusercontent.com/4459398/87717870-571bef80-c76e-11ea-8b7b-1850094326b3.png) 4) Once again, click the `Full screen` button in the table 5) This time, expand an event, which will scroll the view 6) Once again, click the `Exit full screen` button **Expected result** - [x] The global navigation e.g. `Overview Detections Hosts...` is visible above the search bar **Actual result** - [ ] Sometimes, the global navigation e.g. `Overview Detections Hosts...` is **not** visible until the page is scrolled
1 parent 5e8e01f commit dca4a23

File tree

17 files changed

+206
-34
lines changed

17 files changed

+206
-34
lines changed

‎x-pack/plugins/security_solution/common/constants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ export const DEFAULT_INTERVAL_PAUSE = true;
3232
export const DEFAULT_INTERVAL_TYPE = 'manual';
3333
export const DEFAULT_INTERVAL_VALUE = 300000; // ms
3434
export const DEFAULT_TIMEPICKER_QUICK_RANGES = 'timepicker:quickRanges';
35+
export const SCROLLING_DISABLED_CLASS_NAME = 'scrolling-disabled';
3536
export const FILTERS_GLOBAL_HEIGHT = 109; // px
3637
export const FULL_SCREEN_TOGGLED_CLASS_NAME = 'fullScreenToggled';
3738
export const NO_ALERT_INDEX = 'no-alert-index-049FC71A-4C2C-446F-9901-37XMC5024C51';

‎x-pack/plugins/security_solution/public/common/components/add_filter_to_global_search_bar/index.test.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ describe('AddFilterToGlobalSearchBar Component', () => {
4545
);
4646

4747
beforeEach(() => {
48+
jest.useFakeTimers();
4849
store = createStore(
4950
state,
5051
SUB_PLUGINS_REDUCER,
@@ -159,6 +160,8 @@ describe('AddFilterToGlobalSearchBar Component', () => {
159160

160161
wrapper.find('[data-test-subj="withHoverActionsButton"]').simulate('mouseenter');
161162
wrapper.update();
163+
jest.runAllTimers();
164+
wrapper.update();
162165

163166
wrapper
164167
.find('[data-test-subj="hover-actions-container"] [data-euiicon-type]')

‎x-pack/plugins/security_solution/public/common/components/drag_and_drop/draggable_wrapper.test.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ describe('DraggableWrapper', () => {
2222
const message = 'draggable wrapper content';
2323
const mount = useMountAppended();
2424

25+
beforeEach(() => {
26+
jest.useFakeTimers();
27+
});
28+
2529
describe('rendering', () => {
2630
test('it renders against the snapshot', () => {
2731
const wrapper = shallow(
@@ -78,6 +82,8 @@ describe('DraggableWrapper', () => {
7882

7983
wrapper.find('[data-test-subj="withHoverActionsButton"]').simulate('mouseenter');
8084
wrapper.update();
85+
jest.runAllTimers();
86+
wrapper.update();
8187
expect(wrapper.find('[data-test-subj="copy-to-clipboard"]').exists()).toBe(true);
8288
});
8389
});

‎x-pack/plugins/security_solution/public/common/components/drag_and_drop/provider_container.tsx‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -13,13 +13,6 @@ interface ProviderContainerProps {
1313
}
1414

1515
const ProviderContainerComponent = styled.div<ProviderContainerProps>`
16-
&,
17-
&::before,
18-
&::after {
19-
transition: background ${({ theme }) => theme.eui.euiAnimSpeedFast} ease,
20-
color ${({ theme }) => theme.eui.euiAnimSpeedFast} ease;
21-
}
22-
2316
${({ isDragging }) =>
2417
!isDragging &&
2518
css`

‎x-pack/plugins/security_solution/public/common/components/filters_global/filters_global.test.tsx‎

Lines changed: 102 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,20 +4,120 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66

7-
import { shallow } from 'enzyme';
7+
import { mount, ReactWrapper, shallow } from 'enzyme';
88
import React from 'react';
9+
import { StickyContainer } from 'react-sticky';
910

1011
import '../../mock/match_media';
1112
import { FiltersGlobal } from './filters_global';
13+
import { TestProviders } from '../../mock/test_providers';
1214

1315
describe('rendering', () => {
1416
test('renders correctly', () => {
1517
const wrapper = shallow(
16-
<FiltersGlobal>
18+
<FiltersGlobal globalFullScreen={false}>
1719
<p>{'Additional filters here.'}</p>
1820
</FiltersGlobal>
1921
);
2022

2123
expect(wrapper).toMatchSnapshot();
2224
});
25+
26+
describe('full screen mode', () => {
27+
let wrapper: ReactWrapper;
28+
29+
beforeEach(() => {
30+
wrapper = mount(
31+
<TestProviders>
32+
<StickyContainer>
33+
<FiltersGlobal globalFullScreen={true}>
34+
<p>{'Filter content'}</p>
35+
</FiltersGlobal>
36+
</StickyContainer>
37+
</TestProviders>
38+
);
39+
});
40+
41+
test('it does NOT render the sticky container', () => {
42+
expect(wrapper.find('[data-test-subj="sticky-filters-global-container"]').exists()).toBe(
43+
false
44+
);
45+
});
46+
47+
test('it renders the non-sticky container', () => {
48+
expect(wrapper.find('[data-test-subj="non-sticky-global-container"]').exists()).toBe(true);
49+
});
50+
51+
test('it does NOT render the container with a `display: none` style when `show` is true (the default)', () => {
52+
expect(
53+
wrapper.find('[data-test-subj="non-sticky-global-container"]').first()
54+
).not.toHaveStyleRule('display', 'none');
55+
});
56+
});
57+
58+
describe('non-full screen mode', () => {
59+
let wrapper: ReactWrapper;
60+
61+
beforeEach(() => {
62+
wrapper = mount(
63+
<TestProviders>
64+
<StickyContainer>
65+
<FiltersGlobal globalFullScreen={false}>
66+
<p>{'Filter content'}</p>
67+
</FiltersGlobal>
68+
</StickyContainer>
69+
</TestProviders>
70+
);
71+
});
72+
73+
test('it renders the sticky container', () => {
74+
expect(wrapper.find('[data-test-subj="sticky-filters-global-container"]').exists()).toBe(
75+
true
76+
);
77+
});
78+
79+
test('it does NOT render the non-sticky container', () => {
80+
expect(wrapper.find('[data-test-subj="non-sticky-global-container"]').exists()).toBe(false);
81+
});
82+
83+
test('it does NOT render the container with a `display: none` style when `show` is true (the default)', () => {
84+
expect(
85+
wrapper.find('[data-test-subj="sticky-filters-global-container"]').first()
86+
).not.toHaveStyleRule('display', 'none');
87+
});
88+
});
89+
90+
describe('when show is false', () => {
91+
test('in full screen mode it renders the container with a `display: none` style', () => {
92+
const wrapper = mount(
93+
<TestProviders>
94+
<StickyContainer>
95+
<FiltersGlobal globalFullScreen={true} show={false}>
96+
<p>{'Filter content'}</p>
97+
</FiltersGlobal>
98+
</StickyContainer>
99+
</TestProviders>
100+
);
101+
102+
expect(
103+
wrapper.find('[data-test-subj="non-sticky-global-container"]').first()
104+
).toHaveStyleRule('display', 'none');
105+
});
106+
107+
test('in non-full screen mode it renders the container with a `display: none` style', () => {
108+
const wrapper = mount(
109+
<TestProviders>
110+
<StickyContainer>
111+
<FiltersGlobal globalFullScreen={false} show={false}>
112+
<p>{'Filter content'}</p>
113+
</FiltersGlobal>
114+
</StickyContainer>
115+
</TestProviders>
116+
);
117+
118+
expect(
119+
wrapper.find('[data-test-subj="sticky-filters-global-container"]').first()
120+
).toHaveStyleRule('display', 'none');
121+
});
122+
});
23123
});

‎x-pack/plugins/security_solution/public/common/components/filters_global/filters_global.tsx‎

Lines changed: 21 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -47,20 +47,33 @@ const FiltersGlobalContainer = styled.header<{ show: boolean }>`
4747

4848
FiltersGlobalContainer.displayName = 'FiltersGlobalContainer';
4949

50+
const NO_STYLE: React.CSSProperties = {};
51+
5052
export interface FiltersGlobalProps {
5153
children: React.ReactNode;
54+
globalFullScreen: boolean;
5255
show?: boolean;
5356
}
5457

55-
export const FiltersGlobal = React.memo<FiltersGlobalProps>(({ children, show = true }) => (
56-
<Sticky disableCompensation={disableStickyMq.matches} topOffset={-offsetChrome}>
57-
{({ style, isSticky }) => (
58-
<FiltersGlobalContainer show={show}>
59-
<Wrapper className="siemFiltersGlobal" isSticky={isSticky} style={style}>
58+
export const FiltersGlobal = React.memo<FiltersGlobalProps>(
59+
({ children, globalFullScreen, show = true }) =>
60+
globalFullScreen ? (
61+
<FiltersGlobalContainer data-test-subj="non-sticky-global-container" show={show}>
62+
<Wrapper className="siemFiltersGlobal" isSticky={false} style={NO_STYLE}>
6063
{children}
6164
</Wrapper>
6265
</FiltersGlobalContainer>
63-
)}
64-
</Sticky>
65-
));
66+
) : (
67+
<Sticky disableCompensation={disableStickyMq.matches} topOffset={-offsetChrome}>
68+
{({ style, isSticky }) => (
69+
<FiltersGlobalContainer data-test-subj="sticky-filters-global-container" show={show}>
70+
<Wrapper className="siemFiltersGlobal" isSticky={isSticky} style={style}>
71+
{children}
72+
</Wrapper>
73+
</FiltersGlobalContainer>
74+
)}
75+
</Sticky>
76+
)
77+
);
78+
6679
FiltersGlobal.displayName = 'FiltersGlobal';

‎x-pack/plugins/security_solution/public/common/components/page/index.tsx‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
import { EuiBadge, EuiDescriptionList, EuiFlexGroup, EuiIcon, EuiPage } from '@elastic/eui';
88
import styled, { createGlobalStyle } from 'styled-components';
99

10-
import { FULL_SCREEN_TOGGLED_CLASS_NAME } from '../../../../common/constants';
10+
import {
11+
FULL_SCREEN_TOGGLED_CLASS_NAME,
12+
SCROLLING_DISABLED_CLASS_NAME,
13+
} from '../../../../common/constants';
1114

1215
/*
1316
SIDE EFFECT: the following `createGlobalStyle` overrides default styling in angular code that was not theme-friendly
@@ -63,6 +66,14 @@ export const AppGlobalStyle = createGlobalStyle<{ theme: { eui: { euiColorPrimar
6366
.${FULL_SCREEN_TOGGLED_CLASS_NAME} {
6467
${({ theme }) => `background-color: ${theme.eui.euiColorPrimary} !important`};
6568
}
69+
70+
.${SCROLLING_DISABLED_CLASS_NAME} body {
71+
overflow-y: hidden;
72+
}
73+
74+
.${SCROLLING_DISABLED_CLASS_NAME} #kibana-body {
75+
overflow-y: hidden;
76+
}
6677
`;
6778

6879
export const DescriptionListStyled = styled(EuiDescriptionList)`

‎x-pack/plugins/security_solution/public/common/components/with_hover_actions/index.tsx‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,11 @@ import styled from 'styled-components';
1010

1111
import { IS_DRAGGING_CLASS_NAME } from '../drag_and_drop/helpers';
1212

13+
/**
14+
* To avoid expensive changes to the DOM, delay showing the popover menu
15+
*/
16+
const HOVER_INTENT_DELAY = 100; // ms
17+
1318
// eslint-disable-next-line @typescript-eslint/no-explicit-any
1419
const WithHoverActionsPopover = (styled(EuiPopover as any)`
1520
.euiPopover__anchor {
@@ -51,18 +56,27 @@ export const WithHoverActions = React.memo<Props>(
5156
({ alwaysShow = false, closePopOverTrigger, hoverContent, render }) => {
5257
const [isOpen, setIsOpen] = useState(hoverContent != null && alwaysShow);
5358
const [showHoverContent, setShowHoverContent] = useState(false);
59+
const [hoverTimeout, setHoverTimeout] = useState<number | undefined>(undefined);
60+
5461
const onMouseEnter = useCallback(() => {
55-
// NOTE: the following read from the DOM is expensive, but not as
56-
// expensive as the default behavior, which adds a div to the body,
57-
// which-in turn performs a more expensive change to the layout
58-
if (!document.body.classList.contains(IS_DRAGGING_CLASS_NAME)) {
59-
setShowHoverContent(true);
60-
}
61-
}, []);
62+
setHoverTimeout(
63+
Number(
64+
setTimeout(() => {
65+
// NOTE: the following read from the DOM is expensive, but not as
66+
// expensive as the default behavior, which adds a div to the body,
67+
// which-in turn performs a more expensive change to the layout
68+
if (!document.body.classList.contains(IS_DRAGGING_CLASS_NAME)) {
69+
setShowHoverContent(true);
70+
}
71+
}, HOVER_INTENT_DELAY)
72+
)
73+
);
74+
}, [setHoverTimeout, setShowHoverContent]);
6275

6376
const onMouseLeave = useCallback(() => {
77+
clearTimeout(hoverTimeout);
6478
setShowHoverContent(false);
65-
}, []);
79+
}, [hoverTimeout, setShowHoverContent]);
6680

6781
const content = useMemo(
6882
() => (

‎x-pack/plugins/security_solution/public/common/containers/use_full_screen/index.tsx‎

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import { useCallback, useMemo } from 'react';
88
import { useDispatch, useSelector } from 'react-redux';
9+
import { SCROLLING_DISABLED_CLASS_NAME } from '../../../../common/constants';
910

1011
import { inputsSelectors } from '../../store';
1112
import { inputsActions } from '../../store/actions';
@@ -16,7 +17,16 @@ export const useFullScreen = () => {
1617
const timelineFullScreen = useSelector(inputsSelectors.timelineFullScreenSelector) ?? false;
1718

1819
const setGlobalFullScreen = useCallback(
19-
(fullScreen: boolean) => dispatch(inputsActions.setFullScreen({ id: 'global', fullScreen })),
20+
(fullScreen: boolean) => {
21+
if (fullScreen) {
22+
document.body.classList.add(SCROLLING_DISABLED_CLASS_NAME);
23+
} else {
24+
document.body.classList.remove(SCROLLING_DISABLED_CLASS_NAME);
25+
setTimeout(() => window.scrollTo(0, 0), 0);
26+
}
27+
28+
dispatch(inputsActions.setFullScreen({ id: 'global', fullScreen }));
29+
},
2030
[dispatch]
2131
);
2232

‎x-pack/plugins/security_solution/public/detections/pages/detection_engine/detection_engine.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,10 @@ export const DetectionEnginePageComponent: React.FC<PropsFromRedux> = ({
156156
{indicesExist ? (
157157
<StickyContainer>
158158
<EuiWindowEvent event="resize" handler={noop} />
159-
<FiltersGlobal show={showGlobalFilters({ globalFullScreen, graphEventId })}>
159+
<FiltersGlobal
160+
globalFullScreen={globalFullScreen}
161+
show={showGlobalFilters({ globalFullScreen, graphEventId })}
162+
>
160163
<SiemSearchBar id="global" indexPattern={indexPattern} />
161164
</FiltersGlobal>
162165

0 commit comments

Comments
 (0)