fix: check dom element directly to determine if element is fullscreen…

… in order to avoid unnecessary visualization rerenders (#1672) Instead of setting a state to trigger changes related to going fullscreen, the components that need to observe the state of fullscreen will now use the native web api and dom elements. This avoids a second render that was happening when going fullscreen. The onResizeStop listener isn't needed anymore since each plugin individually handles size changes (in MapPlugin, LegacyPlugin, DV Plugin) Some of the files are just function renaming. Other improvement: do not create an ItemContextMenu when in edit mode.
dhis2 · Mar 25, 2021 · 279f01f · 279f01f
1 parent 7703f6b
commit 279f01f
Show file tree

Hide file tree

Showing 16 changed files with 102 additions and 138 deletions.
diff --git a/src/components/Item/VisualizationItem/Item.js b/src/components/Item/VisualizationItem/Item.js
@@ -23,6 +23,8 @@ import { acAddVisualization } from '../../../actions/visualizations'
 import { acSetSelectedItemActiveType } from '../../../actions/selected'
 import { pluginIsAvailable } from './Visualization/plugin'
 import { getDataStatisticsName } from '../../../modules/itemTypes'
+import { isElementFullscreen } from '../../../modules/isElementFullscreen'
+import { getGridItemElement } from '../../../modules/getGridItemElement'
 import { getVisualizationId, getVisualizationName } from '../../../modules/item'
 import memoizeOne from '../../../modules/memoizeOne'
 import {
@@ -36,7 +38,6 @@ export class Item extends Component {
     state = {
         showFooter: false,
         configLoaded: false,
-        isFullscreen: false,
         loadItemFailed: false,
     }
 
@@ -45,7 +46,6 @@ export class Item extends Component {
 
         this.contentRef = React.createRef()
         this.headerRef = React.createRef()
-        this.itemDomElSelector = `.reactgriditem-${this.props.item.id}`
 
         const style = window.getComputedStyle(document.documentElement)
         this.itemContentPadding = parseInt(
@@ -93,45 +93,16 @@ export class Item extends Component {
         }
 
         this.setState({ configLoaded: true })
-
-        const el = document.querySelector(this.itemDomElSelector)
-        if (el?.requestFullscreen) {
-            el.onfullscreenchange = this.handleFullscreenChange
-        } else if (el?.webkitRequestFullscreen) {
-            el.onwebkitfullscreenchange = this.handleFullscreenChange
-        }
-    }
-
-    componentWillUnmount() {
-        const el = document.querySelector(this.itemDomElSelector)
-        if (el?.onfullscreenchange) {
-            el.removeEventListener(
-                'onfullscreenchange',
-                this.handleFullscreenChange
-            )
-        } else if (el?.onwebkitfullscreenchange) {
-            el.removeEventListener(
-                'onwebkitfullscreenchange',
-                this.handleFullscreenChange
-            )
-        }
     }
 
     isFullscreenSupported = () => {
-        const el = document.querySelector(this.itemDomElSelector)
+        const el = getGridItemElement(this.props.item.id)
         return !!(el?.requestFullscreen || el?.webkitRequestFullscreen)
     }
 
-    handleFullscreenChange = () =>
-        this.setState({
-            isFullscreen:
-                !!document.fullscreenElement ||
-                !!document.webkitFullscreenElement,
-        })
-
     onToggleFullscreen = () => {
-        if (!this.state.isFullscreen) {
-            const el = document.querySelector(this.itemDomElSelector)
+        if (!isElementFullscreen(this.props.item.id)) {
+            const el = getGridItemElement(this.props.item.id)
             if (el?.requestFullscreen) {
                 el.requestFullscreen()
             } else if (el?.webkitRequestFullscreen) {
@@ -166,7 +137,7 @@ export class Item extends Component {
     }
 
     getAvailableHeight = ({ width, height }) => {
-        if (this.state.isFullscreen) {
+        if (isElementFullscreen(this.props.item.id)) {
             return (
                 height -
                 this.headerRef.current.clientHeight -
@@ -190,9 +161,9 @@ export class Item extends Component {
     }
 
     getAvailableWidth = () => {
-        const rect = document
-            .querySelector(this.itemDomElSelector)
-            ?.getBoundingClientRect()
+        const rect = getGridItemElement(
+            this.props.item.id
+        )?.getBoundingClientRect()
 
         return rect && rect.width - this.itemContentPadding * 2
     }
@@ -206,20 +177,21 @@ export class Item extends Component {
         const { showFooter } = this.state
         const activeType = this.getActiveType()
 
-        const actionButtons = pluginIsAvailable(activeType || item.type) ? (
-            <ItemContextMenu
-                item={item}
-                visualization={this.props.visualization}
-                onSelectActiveType={this.setActiveType}
-                onToggleFooter={this.onToggleFooter}
-                onToggleFullscreen={this.onToggleFullscreen}
-                activeType={activeType}
-                activeFooter={showFooter}
-                isFullscreen={this.state.isFullscreen}
-                fullscreenSupported={this.isFullscreenSupported()}
-                loadItemFailed={this.state.loadItemFailed}
-            />
-        ) : null
+        const actionButtons =
+            pluginIsAvailable(activeType || item.type) &&
+            isViewMode(dashboardMode) ? (
+                <ItemContextMenu
+                    item={item}
+                    visualization={this.props.visualization}
+                    onSelectActiveType={this.setActiveType}
+                    onToggleFooter={this.onToggleFooter}
+                    onToggleFullscreen={this.onToggleFullscreen}
+                    activeType={activeType}
+                    activeFooter={showFooter}
+                    fullscreenSupported={this.isFullscreenSupported()}
+                    loadItemFailed={this.state.loadItemFailed}
+                />
+            ) : null
 
         return (
             <>
@@ -253,7 +225,6 @@ export class Item extends Component {
                                             dimensions
                                         )}
                                         availableWidth={this.getAvailableWidth()}
-                                        isFullscreen={this.state.isFullscreen}
                                         gridWidth={this.props.gridWidth}
                                     />
                                 )}

diff --git a/src/components/Item/VisualizationItem/ItemContextMenu/ItemContextMenu.js b/src/components/Item/VisualizationItem/ItemContextMenu/ItemContextMenu.js
@@ -13,6 +13,7 @@ import LaunchIcon from '@material-ui/icons/Launch'
 import ViewAsMenuItems from './ViewAsMenuItems'
 import { useWindowDimensions } from '../../../WindowDimensionsProvider'
 import { isSmallScreen } from '../../../../modules/smallScreen'
+import { isElementFullscreen } from '../../../../modules/isElementFullscreen'
 
 import {
     ThreeDots,
@@ -85,9 +86,11 @@ const ItemContextMenu = props => {
 
     const buttonRef = createRef()
 
-    return props.isFullscreen ? (
+    return isElementFullscreen(item.id) ? (
         <Button small secondary onClick={props.onToggleFullscreen}>
-            <ExitFullscreen />
+            <span data-testid="exit-fullscreen-button">
+                <ExitFullscreen />
+            </span>
         </Button>
     ) : (
         <>
@@ -167,7 +170,6 @@ ItemContextMenu.propTypes = {
     activeFooter: PropTypes.bool,
     activeType: PropTypes.string,
     fullscreenSupported: PropTypes.bool,
-    isFullscreen: PropTypes.bool,
     item: PropTypes.object,
     loadItemFailed: PropTypes.bool,
     visualization: PropTypes.object,

diff --git a/src/components/Item/VisualizationItem/ItemContextMenu/__tests__/ItemContextMenu.spec.js b/src/components/Item/VisualizationItem/ItemContextMenu/__tests__/ItemContextMenu.spec.js
@@ -1,9 +1,10 @@
 import React from 'react'
-import { render, waitFor } from '@testing-library/react'
+import { render, waitFor, screen } from '@testing-library/react'
 import { fireEvent } from '@testing-library/dom'
 import WindowDimensionsProvider from '../../../../WindowDimensionsProvider'
 import { useSystemSettings } from '../../../../SystemSettingsProvider'
 import ItemContextMenu from '../ItemContextMenu'
+import { getGridItemDomElementClassName } from '../../../../../modules/getGridItemDomElementClassName'
 
 jest.mock('../../../../SystemSettingsProvider', () => ({
     useSystemSettings: jest.fn(),
@@ -21,6 +22,7 @@ const mockSystemSettingsDefault = {
 const defaultProps = {
     item: {
         type: 'CHART',
+        id: 'rainbowdash',
     },
     visualization: {
         type: 'BAR',
@@ -30,7 +32,6 @@ const defaultProps = {
     activeFooter: false,
     activeType: 'CHART',
     fullscreenSupported: true,
-    isFullscreen: false,
     loadItemFailed: false,
 }
 
@@ -47,17 +48,31 @@ test('renders just the button when menu closed', () => {
 })
 
 test('renders exit fullscreen button', () => {
-    useSystemSettings.mockImplementationOnce(() => mockSystemSettingsDefault)
-    const props = Object.assign({}, defaultProps, {
-        isFullscreen: true,
-    })
-    const { container } = render(
+    useSystemSettings.mockImplementation(() => mockSystemSettingsDefault)
+    const gridItemClassName = getGridItemDomElementClassName(
+        defaultProps.item.id
+    )
+
+    const { rerender } = render(
         <WindowDimensionsProvider>
-            <ItemContextMenu {...props} />
+            <div className={gridItemClassName}>
+                <ItemContextMenu {...defaultProps} />
+            </div>
         </WindowDimensionsProvider>
     )
 
-    expect(container).toMatchSnapshot()
+    document.fullscreenElement = document.querySelector(`.${gridItemClassName}`)
+
+    rerender(
+        <WindowDimensionsProvider>
+            <div className={{ gridItemClassName }}>
+                <ItemContextMenu {...defaultProps} />
+            </div>
+        </WindowDimensionsProvider>
+    )
+
+    document.fullscreenElement = null
+    expect(screen.getByTestId('exit-fullscreen-button')).toBeTruthy()
 })
 
 test('renders popover menu for BAR chart', async () => {

diff --git a/...em/VisualizationItem/ItemContextMenu/__tests__/__snapshots__/ItemContextMenu.spec.js.snap b/...em/VisualizationItem/ItemContextMenu/__tests__/__snapshots__/ItemContextMenu.spec.js.snap
@@ -1,27 +1,5 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
-exports[`renders exit fullscreen button 1`] = `
-<div>
-  <button
-    class="jsx-2371629422 secondary small"
-    data-test="dhis2-uicore-button"
-    type="button"
-  >
-    <svg
-      height="16"
-      viewBox="0 0 16 16"
-      width="16"
-      xmlns="http://www.w3.org/2000/svg"
-    >
-      <path
-        d="m6.5 9c.24545989 0 .44960837.17687516.49194433.41012437l.00805567.08987563v4c0 .2761424-.22385763.5-.5.5-.24545989 0-.44960837-.1768752-.49194433-.4101244l-.00805567-.0898756v-2.794l-3.64644661 3.6475534c-.19526215.1952621-.51184463.1952621-.70710678 0-.17356635-.1735664-.1928515-.4429908-.05785545-.6378589l.05785545-.0692479 3.64555339-3.6464466h-2.792c-.24545989 0-.44960837-.17687516-.49194433-.41012437l-.00805567-.08987563c0-.24545989.17687516-.44960837.41012437-.49194433l.08987563-.00805567zm7.5-.5v5.5h-5.5v-1h4.5v-4.5zm-6.5-6.5v1h-4.5v4.5h-1v-5.5zm6.8535534-.35355339c.1735663.17356635.1928515.44299075.0578554.63785889l-.0578554.06924789-3.6475534 3.64644661h2.794c.2454599 0 .4496084.17687516.4919443.41012437l.0080557.08987563c0 .24545989-.1768752.44960837-.4101244.49194433l-.0898756.00805567h-4c-.24545989 0-.44960837-.17687516-.49194433-.41012437l-.00805567-.08987563v-4c0-.27614237.22385763-.5.5-.5.24545989 0 .44960837.17687516.49194433.41012437l.00805567.08987563v2.792l3.6464466-3.64555339c.1952622-.19526215.5118446-.19526215.7071068 0z"
-        fill="inherit"
-      />
-    </svg>
-  </button>
-</div>
-`;
-
 exports[`renders just the button when menu closed 1`] = `
 <div>
   <div>

diff --git a/src/components/Item/VisualizationItem/Visualization/DefaultPlugin.js b/src/components/Item/VisualizationItem/Visualization/DefaultPlugin.js
@@ -3,7 +3,7 @@ import PropTypes from 'prop-types'
 import { useD2 } from '@dhis2/app-runtime-adapter-d2'
 import { useConfig } from '@dhis2/app-runtime'
 import { load, unmount } from './plugin'
-import getGridItemDomId from '../../../../modules/getGridItemDomId'
+import getVisualizationContainerDomId from '../../../../modules/getVisualizationContainerDomId'
 
 const DefaultPlugin = ({ item, activeType, visualization, options, style }) => {
     const { d2 } = useD2()
@@ -53,7 +53,7 @@ const DefaultPlugin = ({ item, activeType, visualization, options, style }) => {
         return reloadAllowed && visChanged
     }
 
-    return <div id={getGridItemDomId(item.id)} style={style} />
+    return <div id={getVisualizationContainerDomId(item.id)} style={style} />
 }
 
 DefaultPlugin.propTypes = {

diff --git a/src/components/Item/VisualizationItem/Visualization/LegacyPlugin.js b/src/components/Item/VisualizationItem/Visualization/LegacyPlugin.js
@@ -1,25 +1,26 @@
 import React, { useEffect } from 'react'
 import PropTypes from 'prop-types'
 import DefaultPlugin from './DefaultPlugin'
-import getGridItemDomId from '../../../../modules/getGridItemDomId'
+import getVisualizationContainerDomId from '../../../../modules/getVisualizationContainerDomId'
 
 const LegacyPlugin = ({
-    isFullscreen,
     availableHeight,
     availableWidth,
     gridWidth,
     ...props
 }) => {
     useEffect(() => {
-        const el = document.querySelector(`#${getGridItemDomId(props.item.id)}`)
+        const el = document.querySelector(
+            `#${getVisualizationContainerDomId(props.item.id)}`
+        )
         if (typeof el?.setViewportSize === 'function') {
             setTimeout(
                 () =>
                     el.setViewportSize(el.clientWidth - 5, el.clientHeight - 5),
                 10
             )
         }
-    }, [availableHeight, availableWidth, isFullscreen, gridWidth])
+    }, [availableHeight, availableWidth, gridWidth])
 
     return <DefaultPlugin {...props} />
 }

diff --git a/src/components/Item/VisualizationItem/Visualization/MapPlugin.js b/src/components/Item/VisualizationItem/Visualization/MapPlugin.js
@@ -3,20 +3,20 @@ import PropTypes from 'prop-types'
 import i18n from '@dhis2/d2-i18n'
 import DefaultPlugin from './DefaultPlugin'
 import { MAP } from '../../../../modules/itemTypes'
+import { isElementFullscreen } from '../../../../modules/isElementFullscreen'
 import { pluginIsAvailable, resize } from './plugin'
 import NoVisualizationMessage from './NoVisualizationMessage'
 
 const MapPlugin = ({
     applyFilters,
-    isFullscreen,
     availableHeight,
     availableWidth,
     gridWidth,
     ...props
 }) => {
     useEffect(() => {
-        resize(props.item.id, MAP, isFullscreen)
-    }, [availableHeight, availableWidth, isFullscreen, gridWidth])
+        resize(props.item.id, MAP, isElementFullscreen(props.item.id))
+    }, [availableHeight, availableWidth, gridWidth])
 
     if (props.item.type === MAP) {
         // apply filters only to thematic and event layers

diff --git a/src/components/Item/VisualizationItem/Visualization/plugin.js b/src/components/Item/VisualizationItem/Visualization/plugin.js
@@ -7,7 +7,7 @@ import {
     itemTypeMap,
 } from '../../../../modules/itemTypes'
 import { getVisualizationId } from '../../../../modules/item'
-import getGridItemDomId from '../../../../modules/getGridItemDomId'
+import getVisualizationContainerDomId from '../../../../modules/getVisualizationContainerDomId'
 import { loadExternalScript } from '../../../../modules/loadExternalScript'
 
 //external plugins
@@ -101,7 +101,7 @@ export const load = async (
     const config = {
         ...visualization,
         ...options,
-        el: getGridItemDomId(item.id),
+        el: getVisualizationContainerDomId(item.id),
     }
 
     const type = activeType || item.type
@@ -111,14 +111,14 @@ export const load = async (
 export const resize = async (id, type, isFullscreen = false) => {
     const plugin = await getPlugin(type)
     if (plugin?.resize) {
-        plugin.resize(getGridItemDomId(id), isFullscreen)
+        plugin.resize(getVisualizationContainerDomId(id), isFullscreen)
     }
 }
 
 export const unmount = async (item, activeType) => {
     const plugin = await getPlugin(activeType)
 
     if (plugin && plugin.unmount) {
-        plugin.unmount(getGridItemDomId(item.id))
+        plugin.unmount(getVisualizationContainerDomId(item.id))
     }
 }
diff --git a/src/components/Item/VisualizationItem/__tests__/__snapshots__/Item.spec.js.snap b/src/components/Item/VisualizationItem/__tests__/__snapshots__/Item.spec.js.snap
@@ -8,7 +8,6 @@ exports[`VisualizationItem/Item does not render Visualization if config not load
         activeFooter={false}
         activeType="CHART"
         fullscreenSupported={false}
-        isFullscreen={false}
         item={
           Object {
             "chart": Object {