Skip to content
This repository has been archived by the owner on Oct 16, 2024. It is now read-only.

Fix issue with preview width when window is resized #109

Merged
merged 3 commits into from
Mar 14, 2023
Merged

Fix issue with preview width when window is resized #109

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
088835f
Create useWindowResize hook
mike-day Mar 13, 2023
e403257
In PatternPreview, use derived state for previewContainerSize; remove…
mike-day Mar 13, 2023
434f842
Use the new useWindowResize hook in PatternGrid
mike-day Mar 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 3 additions & 10 deletions wp-modules/app/js/src/components/PatternPreview/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// WP dependencies
import { useState, useRef, useEffect } from '@wordpress/element';
import { useRef } from '@wordpress/element';
import { __ } from '@wordpress/i18n';

// External dependencies
Expand All @@ -21,20 +21,13 @@ export default function PatternPreview( {
url,
viewportWidth,
}: PatternPreviewProps ) {
const [ previewContainerSize, setPreviewContainerSize ] =
useState< BoundingClientRect >();
const previewContainer = useRef< HTMLDivElement | null >( null );
const { lazyHasIntersected } = useLazyRender( previewContainer, {
threshold: [ 0.3, 0.6, 1.0 ],
} );

useEffect( () => {
if ( previewContainer?.current ) {
setPreviewContainerSize(
previewContainer?.current?.getBoundingClientRect()
);
}
}, [ previewContainer ] );
const previewContainerSize: BoundingClientRect | undefined =
previewContainer?.current?.getBoundingClientRect();
Comment on lines -31 to +30
Copy link
Contributor Author

@mike-day mike-day Mar 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Derived state needs to be used for previewContainerSize for this idea to work.

With it, a fresh getBoundingClientRect() is called each time the component is rendered.

Otherwise, useEffect is not called again after initial render since the previewContainer dependency does not update, making it harder to call setPreviewContainerSize with the new bounding client rect.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea to use derived state


const scale = previewContainerSize
? previewContainerSize?.width / viewportWidth
Expand Down
7 changes: 7 additions & 0 deletions wp-modules/app/js/src/components/Patterns/PatternGrid.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ import loadable from '@loadable/component';
// Globals
import { patternManager } from '../../globals';

// Hooks
import useWindowResize from '../../hooks/useWindowResize';

// Components
const PatternPreview: PatternPreviewType = loadable(
async () => import( '../PatternPreview' )
Expand All @@ -26,6 +29,10 @@ type Props = {

/** Render the patterns in a grid, or a message if no patterns are found. */
export default function PatternGrid( { themePatterns }: Props ) {
// If the window is resized, trigger a fresh render of the grid.
// Helps ensure PatternPreview iFrames are the right size.
useWindowResize();
Comment on lines +32 to +34
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useWindowResize is called in PatternGrid so only the grid is re-rendered (instead of calling higher in the tree and also re-rendering categories, for example).

Copy link
Contributor

@kienstra kienstra Mar 14, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really good idea to keep re-rendering to this low scope


return (
<>
{ ! Object.entries( themePatterns ?? {} ).length ? (
Expand Down
16 changes: 16 additions & 0 deletions wp-modules/app/js/src/hooks/useWindowResize.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
import { useLayoutEffect, useState } from '@wordpress/element';

/** Rerender the calling component when the window is resized. */
export default function useWindowResize() {
const [ , setWindowSize ] = useState( [ 0, 0 ] );
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also add a windowSize const and return it if we wanted to use that data elsewhere, but I see no practical usage for it right now.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a nice trick to force a re-render.

The fact that there's a no windowSize makes it clear that we're not using it, we're just using setState() to force a re-render.


useLayoutEffect( () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice use of useLayoutEffect() instead of useEffect()

function updateSizeAndRerender() {
setWindowSize( [ window.innerWidth, window.innerHeight ] );
}

window.addEventListener( 'resize', updateSizeAndRerender );
return () =>
window.removeEventListener( 'resize', updateSizeAndRerender );
}, [] );
}