mirror of
https://github.com/elastic/kibana.git
synced 2025-04-23 09:19:04 -04:00
[Security Solution] Fix tab navigation scrolling to top (#166655)
## Summary issue: https://github.com/elastic/kibana/issues/163606 Fixes the bug on the Security tab navigation component scrolling to the top of the page when clicked. ### Problem As described in the React Router [docs](https://v5.reactrouter.com/web/guides/scroll-restoration), all browsers introduced scroll reset in the `history.pushState` API ([spec](https://majido.github.io/scroll-restoration-proposal/history-based-api.html#web-idl)), which is the expected behavior on most of the regular navigation actions, however in some cases such as the tabs navigation we don't want the scrolling to be reset to the top after pushing the URL. The spec also defines the `window.history.scrollRestoration` API to configure this behavior ('manual' or 'auto'), but it does not work correctly when using `react-route-dom` history object to navigate. React Router v5 also provides a `ScrollRestoration` component to solve this problem, but it only works with _"data router"_ type, we use _"browser router"_ in Kibana, which is not supported by this tool. There are other libraries that also solve this problem, implementing a very similar approach, such as [react-scroll-restoration](https://github.com/nanyang24/react-scroll-restoration). ### Solution Instead of loading new external modules, I implemented the same approach integrating it into our Security navigation tools: The `navigateTo` function now accepts a new `restoreScroll` option prop, when set to `true` it will prevent the scroll reset after navigating by restoring the previous scroll position when it receives the event triggered by the Browser. In the case of using an old browser version that does not trigger the scroll reset after the navigation, this change will have no implication other than losing the first scroll event after the navigation, this will be very hard to notice since many events are sent when a user scrolls manually. ## Recordings Before:464f708a
-f373-47b9-9826-a5cfa7473f5b After:46d99b12
-69e8-410a-a700-ca9ef7f5c2a4 Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
This commit is contained in:
parent
84cfac7aee
commit
f8927dd1fe
3 changed files with 38 additions and 2 deletions
|
@ -7,6 +7,7 @@
|
|||
import { useGetAppUrl, useNavigateTo } from './navigation';
|
||||
import { mockGetUrlForApp, mockNavigateToApp, mockNavigateToUrl } from '../mocks/context';
|
||||
import { renderHook } from '@testing-library/react-hooks';
|
||||
import { fireEvent } from '@testing-library/dom';
|
||||
|
||||
jest.mock('./context');
|
||||
|
||||
|
@ -61,5 +62,22 @@ describe('yourFile', () => {
|
|||
expect(mockNavigateToApp).not.toHaveBeenCalled();
|
||||
expect(mockNavigateToUrl).toHaveBeenCalledWith(URL);
|
||||
});
|
||||
|
||||
it('navigates restoring the scroll', async () => {
|
||||
const { result } = renderHook(useNavigateTo);
|
||||
const { navigateTo } = result.current;
|
||||
|
||||
const currentScrollY = 100;
|
||||
window.scrollY = currentScrollY;
|
||||
window.scrollTo = jest.fn();
|
||||
|
||||
navigateTo({ url: URL, restoreScroll: true });
|
||||
|
||||
// Simulates the browser scroll reset event
|
||||
fireEvent(window, new Event('scroll'));
|
||||
|
||||
expect(window.scrollTo).toHaveBeenCalledTimes(1);
|
||||
expect(window.scrollTo).toHaveBeenCalledWith(0, currentScrollY);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
@ -34,6 +34,11 @@ export type NavigateTo = (
|
|||
param: {
|
||||
url?: string;
|
||||
appId?: string;
|
||||
/**
|
||||
* Browsers will reset the scroll position to 0 when navigating to a new page.
|
||||
* This option will prevent that from happening.
|
||||
*/
|
||||
restoreScroll?: boolean;
|
||||
} & NavigateToAppOptions
|
||||
) => void;
|
||||
/**
|
||||
|
@ -44,7 +49,10 @@ export const useNavigateTo = () => {
|
|||
const { navigateToApp, navigateToUrl } = useNavigationContext().application;
|
||||
|
||||
const navigateTo = useCallback<NavigateTo>(
|
||||
({ url, appId = SECURITY_UI_APP_ID, ...options }) => {
|
||||
({ url, appId = SECURITY_UI_APP_ID, restoreScroll, ...options }) => {
|
||||
if (restoreScroll) {
|
||||
addScrollRestoration();
|
||||
}
|
||||
if (url) {
|
||||
navigateToUrl(url);
|
||||
} else {
|
||||
|
@ -56,6 +64,16 @@ export const useNavigateTo = () => {
|
|||
return { navigateTo };
|
||||
};
|
||||
|
||||
/**
|
||||
* Expects the browser scroll reset event to be fired after the navigation,
|
||||
* then restores the previous scroll position.
|
||||
*/
|
||||
const addScrollRestoration = () => {
|
||||
const scrollY = window.scrollY;
|
||||
const handler = () => window.scrollTo(0, scrollY);
|
||||
window.addEventListener('scroll', handler, { once: true });
|
||||
};
|
||||
|
||||
/**
|
||||
* Returns `navigateTo` and `getAppUrl` navigation hooks
|
||||
*/
|
||||
|
|
|
@ -31,7 +31,7 @@ const TabNavigationItemComponent = ({
|
|||
const handleClick = useCallback(
|
||||
(ev) => {
|
||||
ev.preventDefault();
|
||||
navigateTo({ path: hrefWithSearch });
|
||||
navigateTo({ path: hrefWithSearch, restoreScroll: true });
|
||||
track(METRIC_TYPE.CLICK, `${TELEMETRY_EVENT.TAB_CLICKED}${id}`);
|
||||
},
|
||||
[navigateTo, hrefWithSearch, id]
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue