From c69e5b8d77f63405b54212e663147804c0147f6f Mon Sep 17 00:00:00 2001 From: Nathan Reese Date: Mon, 28 Oct 2024 11:21:25 -0600 Subject: [PATCH] do not set full screen mode on ExitFullScreenButton re-render (#198012) ### Background https://github.com/elastic/kibana/pull/194892 is refactoring [DashboardRenderer](https://github.com/elastic/kibana/blob/3391344e8dc8377d359b918521b6c48838cde8ae/src/plugins/dashboard/public/dashboard_container/external_api/dashboard_renderer.tsx) component to replace Dashboard Embeddable with a plain old javascript object. Dashboard Embeddable rendered its contents in a new react tree. The new implementation does not. Since the new implementation does not render the dashboard in a new react tree, any re-render in `DashboardViewport` parent components causes `ExitFullScreenButton` to re-render. In its current form, re-rendering `ExitFullScreenButton` calls `onExit`, which causing dashboard to exit full screen mode. This PR makes use of `useCallback` to fix the issue where re-rending `ExitFullScreenButton` calls `onExit`. ### Test steps 1) Open dashboard that ships with sample web logs data set 2) switch to view mode 3) click "Full screen" button 4) Maximize a panel. Verify dashboard stays in full screen mode. --- .../button/exit_full_screen/src/services.tsx | 12 +++-- .../component/viewport/dashboard_viewport.tsx | 46 +++++-------------- 2 files changed, 19 insertions(+), 39 deletions(-) diff --git a/packages/shared-ux/button/exit_full_screen/src/services.tsx b/packages/shared-ux/button/exit_full_screen/src/services.tsx index a167b2b116bf0..9497a6ed34468 100644 --- a/packages/shared-ux/button/exit_full_screen/src/services.tsx +++ b/packages/shared-ux/button/exit_full_screen/src/services.tsx @@ -7,7 +7,7 @@ * License v3.0 only", or the "Server Side Public License, v 1". */ -import React, { FC, useContext, PropsWithChildren } from 'react'; +import React, { FC, useContext, PropsWithChildren, useCallback } from 'react'; import type { Services, @@ -37,12 +37,16 @@ export const ExitFullScreenButtonProvider: FC > = ({ children, ...services }) => { + const setIsFullscreen = useCallback( + (isFullscreen: boolean) => { + services.coreStart.chrome.setIsVisible(!isFullscreen); + }, + [services.coreStart.chrome] + ); return ( { - services.coreStart.chrome.setIsVisible(!isFullscreen); - }, + setIsFullscreen, customBranding$: services.coreStart.customBranding.customBranding$, }} > diff --git a/src/plugins/dashboard/public/dashboard_container/component/viewport/dashboard_viewport.tsx b/src/plugins/dashboard/public/dashboard_container/component/viewport/dashboard_viewport.tsx index 664a3c43a8d9d..027d2aee62b15 100644 --- a/src/plugins/dashboard/public/dashboard_container/component/viewport/dashboard_viewport.tsx +++ b/src/plugins/dashboard/public/dashboard_container/component/viewport/dashboard_viewport.tsx @@ -10,7 +10,7 @@ import { debounce } from 'lodash'; import classNames from 'classnames'; import useResizeObserver from 'use-resize-observer/polyfilled'; -import React, { useEffect, useMemo, useState } from 'react'; +import React, { useCallback, useEffect, useMemo, useState } from 'react'; import { EuiPortal } from '@elastic/eui'; import { ReactEmbeddableRenderer, ViewMode } from '@kbn/embeddable-plugin/public'; @@ -22,10 +22,7 @@ import { ControlGroupSerializedState, } from '@kbn/controls-plugin/public'; import { CONTROL_GROUP_TYPE } from '@kbn/controls-plugin/common'; -import { - useBatchedPublishingSubjects, - useStateFromPublishingSubject, -} from '@kbn/presentation-publishing'; +import { useBatchedPublishingSubjects } from '@kbn/presentation-publishing'; import { DashboardGrid } from '../grid'; import { useDashboardApi } from '../../../dashboard_api/use_dashboard_api'; import { DashboardEmptyScreen } from '../empty_screen/dashboard_empty_screen'; @@ -44,7 +41,7 @@ export const useDebouncedWidthObserver = (skipDebounce = false, wait = 100) => { return { ref, width }; }; -export const DashboardViewportComponent = () => { +export const DashboardViewport = () => { const dashboardApi = useDashboardApi(); const [hasControls, setHasControls] = useState(false); const [ @@ -70,6 +67,9 @@ export const DashboardViewportComponent = () => { dashboardApi.uuid$, dashboardApi.fullScreenMode$ ); + const onExit = useCallback(() => { + dashboardApi.setFullScreenMode(false); + }, [dashboardApi]); const panelCount = useMemo(() => { return Object.keys(panels).length; @@ -142,6 +142,11 @@ export const DashboardViewportComponent = () => { /> ) : null} + {fullScreenMode && ( + + + + )} {panelCount === 0 && }
{
); }; - -// This fullscreen button HOC separates fullscreen button and dashboard content to reduce rerenders -// because ExitFullScreenButton sets isFullscreenMode to false on unmount while rerendering. -// This specifically fixed maximizing/minimizing panels without exiting fullscreen mode. -const WithFullScreenButton = ({ children }: { children: JSX.Element }) => { - const dashboardApi = useDashboardApi(); - - const isFullScreenMode = useStateFromPublishingSubject(dashboardApi.fullScreenMode$); - - return ( - <> - {children} - {isFullScreenMode && ( - - dashboardApi.setFullScreenMode(false)} - toggleChrome={!dashboardApi.isEmbeddedExternally} - /> - - )} - - ); -}; - -export const DashboardViewport = () => ( - - - -);