From 9d3ed5339d447ebd6b22604d0eb304001a951d0b Mon Sep 17 00:00:00 2001 From: Chiaki Obara Date: Wed, 23 Jun 2021 20:21:23 +0900 Subject: [PATCH 1/6] Replace FullscreenMode from Class comopnent to functional components (with test) --- .../src/components/fullscreen-mode/index.js | 63 +++++++++---------- .../components/fullscreen-mode/test/index.js | 41 +++++++----- 2 files changed, 54 insertions(+), 50 deletions(-) diff --git a/packages/interface/src/components/fullscreen-mode/index.js b/packages/interface/src/components/fullscreen-mode/index.js index fc3079b1d1434b..ba2c1a69d1e3d9 100644 --- a/packages/interface/src/components/fullscreen-mode/index.js +++ b/packages/interface/src/components/fullscreen-mode/index.js @@ -1,51 +1,48 @@ /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; +import { useEffect, useState } from '@wordpress/element'; -export class FullscreenMode extends Component { - componentDidMount() { - this.isSticky = false; - this.sync(); +export const FullscreenMode = ( { isActive } ) => { + const [ isSticky, setIsSticky ] = useState( false ); + // componentDidMount + useEffect( () => { // `is-fullscreen-mode` is set in PHP as a body class by Gutenberg, and this causes // `sticky-menu` to be applied by WordPress and prevents the admin menu being scrolled // even if `is-fullscreen-mode` is then removed. Let's remove `sticky-menu` here as // a consequence of the FullscreenMode setup if ( document.body.classList.contains( 'sticky-menu' ) ) { - this.isSticky = true; + setIsSticky( true ); document.body.classList.remove( 'sticky-menu' ); } - } - - componentWillUnmount() { - if ( this.isSticky ) { - document.body.classList.add( 'sticky-menu' ); - } - - if ( this.props.isActive ) { - document.body.classList.remove( 'is-fullscreen-mode' ); - } - } - - componentDidUpdate( prevProps ) { - if ( this.props.isActive !== prevProps.isActive ) { - this.sync(); - } - } - - sync() { - const { isActive } = this.props; + }, [ setIsSticky ] ); + + // componentWillUnmount + useEffect( () => { + return () => { + if ( isSticky ) { + document.body.classList.add( 'sticky-menu' ); + } + }; + }, [ isSticky ] ); + + useEffect( () => { + return () => { + if ( isActive ) { + document.body.classList.remove( 'is-fullscreen-mode' ); + } + }; + }, [ isActive ] ); + + // componentDidUpdate + useEffect( () => { if ( isActive ) { document.body.classList.add( 'is-fullscreen-mode' ); } else { document.body.classList.remove( 'is-fullscreen-mode' ); } - } - - render() { - return null; - } -} + }, [ isActive ] ); -export default FullscreenMode; + return null; +}; diff --git a/packages/interface/src/components/fullscreen-mode/test/index.js b/packages/interface/src/components/fullscreen-mode/test/index.js index 06f49ec16644d8..5904a16e11f0b2 100644 --- a/packages/interface/src/components/fullscreen-mode/test/index.js +++ b/packages/interface/src/components/fullscreen-mode/test/index.js @@ -1,8 +1,7 @@ /** * External dependencies */ -import { shallow } from 'enzyme'; - +import { act, create } from 'react-test-renderer'; /** * Internal dependencies */ @@ -10,16 +9,18 @@ import { FullscreenMode } from '..'; describe( 'FullscreenMode', () => { it( 'fullscreen mode to be added to document body when active', () => { - shallow( ); - + act( () => { + create( ); + } ); expect( document.body.classList.contains( 'is-fullscreen-mode' ) ).toBe( true ); } ); it( 'fullscreen mode not to be added to document body when active', () => { - shallow( ); - + act( () => { + create( ); + } ); expect( document.body.classList.contains( 'is-fullscreen-mode' ) ).toBe( false ); @@ -27,9 +28,9 @@ describe( 'FullscreenMode', () => { it( 'sticky-menu to be removed from the body class if present', () => { document.body.classList.add( 'sticky-menu' ); - - shallow( ); - + act( () => { + create( ); + } ); expect( document.body.classList.contains( 'sticky-menu' ) ).toBe( false ); @@ -37,10 +38,13 @@ describe( 'FullscreenMode', () => { it( 'sticky-menu to be restored when component unmounted and originally present', () => { document.body.classList.add( 'sticky-menu' ); - - const mode = shallow( ); - mode.unmount(); - + let mode; + act( () => { + mode = create( ); + } ); + act( () => { + mode.unmount(); + } ); expect( document.body.classList.contains( 'sticky-menu' ) ).toBe( true ); @@ -51,15 +55,18 @@ describe( 'FullscreenMode', () => { expect( document.body.classList.contains( 'is-fullscreen-mode' ) ).toBe( false ); - - const mode = shallow( ); - + let mode; + act( () => { + mode = create( ); + } ); // present after mounting with `isActive` expect( document.body.classList.contains( 'is-fullscreen-mode' ) ).toBe( true ); - mode.unmount(); + act( () => { + mode.unmount(); + } ); // removed after unmounting expect( document.body.classList.contains( 'is-fullscreen-mode' ) ).toBe( From 7c224058a4547fc5ca4121264e0c330789cc4469 Mon Sep 17 00:00:00 2001 From: Chiaki Obara Date: Thu, 24 Jun 2021 11:03:56 +0900 Subject: [PATCH 2/6] export default set --- packages/interface/src/components/fullscreen-mode/index.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/interface/src/components/fullscreen-mode/index.js b/packages/interface/src/components/fullscreen-mode/index.js index ba2c1a69d1e3d9..d9683fc7fa02e6 100644 --- a/packages/interface/src/components/fullscreen-mode/index.js +++ b/packages/interface/src/components/fullscreen-mode/index.js @@ -3,7 +3,7 @@ */ import { useEffect, useState } from '@wordpress/element'; -export const FullscreenMode = ( { isActive } ) => { +const FullscreenMode = ( { isActive } ) => { const [ isSticky, setIsSticky ] = useState( false ); // componentDidMount @@ -46,3 +46,4 @@ export const FullscreenMode = ( { isActive } ) => { return null; }; +export default FullscreenMode; From 6e0257032271a50f859aec01040f0c4d9527b70e Mon Sep 17 00:00:00 2001 From: Chiaki Obara Date: Thu, 24 Jun 2021 11:56:55 +0900 Subject: [PATCH 3/6] fix import --- packages/interface/src/components/fullscreen-mode/test/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/interface/src/components/fullscreen-mode/test/index.js b/packages/interface/src/components/fullscreen-mode/test/index.js index 5904a16e11f0b2..35d889a9ad5217 100644 --- a/packages/interface/src/components/fullscreen-mode/test/index.js +++ b/packages/interface/src/components/fullscreen-mode/test/index.js @@ -5,7 +5,7 @@ import { act, create } from 'react-test-renderer'; /** * Internal dependencies */ -import { FullscreenMode } from '..'; +import FullscreenMode from '..'; describe( 'FullscreenMode', () => { it( 'fullscreen mode to be added to document body when active', () => { From cdf00a33b32c347a3d28f71b82287da761fb5c7c Mon Sep 17 00:00:00 2001 From: Chiaki Obara Date: Thu, 24 Jun 2021 19:01:59 +0900 Subject: [PATCH 4/6] change useStatus to useRef --- .../src/components/fullscreen-mode/index.js | 27 ++++++++----------- 1 file changed, 11 insertions(+), 16 deletions(-) diff --git a/packages/interface/src/components/fullscreen-mode/index.js b/packages/interface/src/components/fullscreen-mode/index.js index d9683fc7fa02e6..3e0599481cc3f0 100644 --- a/packages/interface/src/components/fullscreen-mode/index.js +++ b/packages/interface/src/components/fullscreen-mode/index.js @@ -1,47 +1,42 @@ /** * WordPress dependencies */ -import { useEffect, useState } from '@wordpress/element'; +import { useEffect, useRef } from '@wordpress/element'; const FullscreenMode = ( { isActive } ) => { - const [ isSticky, setIsSticky ] = useState( false ); + const isSticky = useRef( false ); - // componentDidMount useEffect( () => { // `is-fullscreen-mode` is set in PHP as a body class by Gutenberg, and this causes // `sticky-menu` to be applied by WordPress and prevents the admin menu being scrolled // even if `is-fullscreen-mode` is then removed. Let's remove `sticky-menu` here as // a consequence of the FullscreenMode setup if ( document.body.classList.contains( 'sticky-menu' ) ) { - setIsSticky( true ); + isSticky.current = true; document.body.classList.remove( 'sticky-menu' ); } - }, [ setIsSticky ] ); + }, [] ); - // componentWillUnmount useEffect( () => { return () => { - if ( isSticky ) { + if ( isSticky.current ) { document.body.classList.add( 'sticky-menu' ); } }; }, [ isSticky ] ); - useEffect( () => { - return () => { - if ( isActive ) { - document.body.classList.remove( 'is-fullscreen-mode' ); - } - }; - }, [ isActive ] ); - - // componentDidUpdate useEffect( () => { if ( isActive ) { document.body.classList.add( 'is-fullscreen-mode' ); } else { document.body.classList.remove( 'is-fullscreen-mode' ); } + + return () => { + if ( isActive ) { + document.body.classList.remove( 'is-fullscreen-mode' ); + } + }; }, [ isActive ] ); return null; From 65138084f03e2af02cdd567df311051b40f97727 Mon Sep 17 00:00:00 2001 From: Chiaki Obara Date: Thu, 24 Jun 2021 21:07:40 +0900 Subject: [PATCH 5/6] fix the dependency --- packages/interface/src/components/fullscreen-mode/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/interface/src/components/fullscreen-mode/index.js b/packages/interface/src/components/fullscreen-mode/index.js index 3e0599481cc3f0..0eec78484994f5 100644 --- a/packages/interface/src/components/fullscreen-mode/index.js +++ b/packages/interface/src/components/fullscreen-mode/index.js @@ -15,15 +15,13 @@ const FullscreenMode = ( { isActive } ) => { isSticky.current = true; document.body.classList.remove( 'sticky-menu' ); } - }, [] ); - useEffect( () => { return () => { if ( isSticky.current ) { document.body.classList.add( 'sticky-menu' ); } }; - }, [ isSticky ] ); + }, [] ); useEffect( () => { if ( isActive ) { From b7fa726fe88f1a24f796d00cff0e10151338bc36 Mon Sep 17 00:00:00 2001 From: Chiaki Obara Date: Fri, 25 Jun 2021 11:41:17 +0900 Subject: [PATCH 6/6] useRef change let --- .../interface/src/components/fullscreen-mode/index.js | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/interface/src/components/fullscreen-mode/index.js b/packages/interface/src/components/fullscreen-mode/index.js index 0eec78484994f5..e9611b9c6e3680 100644 --- a/packages/interface/src/components/fullscreen-mode/index.js +++ b/packages/interface/src/components/fullscreen-mode/index.js @@ -1,23 +1,22 @@ /** * WordPress dependencies */ -import { useEffect, useRef } from '@wordpress/element'; +import { useEffect } from '@wordpress/element'; const FullscreenMode = ( { isActive } ) => { - const isSticky = useRef( false ); - useEffect( () => { + let isSticky = false; // `is-fullscreen-mode` is set in PHP as a body class by Gutenberg, and this causes // `sticky-menu` to be applied by WordPress and prevents the admin menu being scrolled // even if `is-fullscreen-mode` is then removed. Let's remove `sticky-menu` here as // a consequence of the FullscreenMode setup if ( document.body.classList.contains( 'sticky-menu' ) ) { - isSticky.current = true; + isSticky = true; document.body.classList.remove( 'sticky-menu' ); } return () => { - if ( isSticky.current ) { + if ( isSticky ) { document.body.classList.add( 'sticky-menu' ); } };