From 0cda399673495ef06514ec02c5034bc67d2631e2 Mon Sep 17 00:00:00 2001 From: Ashar Fuadi Date: Tue, 20 Jun 2023 14:44:51 +0700 Subject: [PATCH 1/4] Site Assembler: ensure only one snackbar notice is shown per unique action --- .../internals/steps-repository/pattern-assembler/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx index 90ed32ee337e8d..743d3dd43f44fd 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx @@ -213,7 +213,8 @@ const PatternAssembler = ( { }; const showNotice = ( action: string, pattern: Pattern ) => { - noticeOperations.createNotice( { content: getNoticeContent( action, pattern ) } ); + noticeOperations.removeNotice( action ); + noticeOperations.createNotice( { id: action, content: getNoticeContent( action, pattern ) } ); }; const getDesign = () => From fd4151c99068a840c5d293545f0a0eb0b8ba5ba1 Mon Sep 17 00:00:00 2001 From: Ashar Fuadi Date: Wed, 21 Jun 2023 11:30:23 +0700 Subject: [PATCH 2/4] Reset timer for a notice with new content --- .../pattern-assembler/notices/notices.tsx | 40 ++++++++++--------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx index f6d3445e08fac9..9f9a20b11830a2 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx @@ -1,37 +1,39 @@ -import { SnackbarList, withNotices, NoticeList } from '@wordpress/components'; +import { SnackbarList, withNotices } from '@wordpress/components'; import i18n from 'i18n-calypso'; -import { useEffect } from 'react'; +import { ReactNode, useEffect, useRef } from 'react'; import type { Pattern } from '../types'; import './notices.scss'; const NOTICE_TIMEOUT = 5000; -type Notice = NoticeList.Notice & { - timer?: ReturnType< typeof setTimeout >; -}; - const Notices = ( { noticeList, noticeOperations, }: Pick< withNotices.Props, 'noticeList' | 'noticeOperations' > ) => { const onRemoveNotice = ( id: string ) => { - const notice = noticeList.find( ( notice ) => id === notice.id ) as Notice; - if ( notice?.timer ) { - clearTimeout( notice.timer ); - delete notice.timer; - } noticeOperations.removeNotice( id ); }; + const timersByContentRef = useRef< Map< ReactNode, ReturnType< typeof setTimeout > > >( + new Map() + ); useEffect( () => { - const lastNotice = noticeList[ noticeList.length - 1 ] as Notice; - - if ( lastNotice?.id && ! lastNotice?.timer ) { - lastNotice.timer = setTimeout( - () => noticeOperations.removeNotice( lastNotice.id ), - NOTICE_TIMEOUT - ); - } + timersByContentRef.current.forEach( ( timer, content ) => { + if ( ! noticeList.some( ( notice ) => notice.content === content ) ) { + clearTimeout( timer ); + timersByContentRef.current.delete( content ); + } + } ); + noticeList.forEach( ( notice ) => { + if ( ! timersByContentRef.current.has( notice.content ) ) { + timersByContentRef.current.set( + notice.content, + setTimeout( () => { + noticeOperations.removeNotice( notice.id ); + }, NOTICE_TIMEOUT ) + ); + } + } ); }, [ noticeList, noticeOperations ] ); return ; From 3961f475896cb2c28b240fabfd16eff4a042113d Mon Sep 17 00:00:00 2001 From: Ashar Fuadi Date: Wed, 21 Jun 2023 14:20:54 +0700 Subject: [PATCH 3/4] Reimplement withNotices --- .../pattern-assembler/index.tsx | 34 ++--- .../pattern-assembler/notices/notices.tsx | 141 ++++++++++++------ 2 files changed, 106 insertions(+), 69 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx index 743d3dd43f44fd..1c2e1563d1eee8 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/index.tsx @@ -6,7 +6,6 @@ import { __experimentalNavigatorProvider as NavigatorProvider, __experimentalNavigatorScreen as NavigatorScreen, __experimentalUseNavigator as useNavigator, - withNotices, } from '@wordpress/components'; import { compose } from '@wordpress/compose'; import { useDispatch, useSelect } from '@wordpress/data'; @@ -33,7 +32,7 @@ import usePatternCategories from './hooks/use-pattern-categories'; import usePatternsMapByCategory from './hooks/use-patterns-map-by-category'; import { usePrefetchImages } from './hooks/use-prefetch-images'; import useRecipe from './hooks/use-recipe'; -import Notices, { getNoticeContent } from './notices/notices'; +import withNotices, { NoticesProps } from './notices/notices'; import PatternAssemblerContainer from './pattern-assembler-container'; import PatternLargePreview from './pattern-large-preview'; import ScreenActivation from './screen-activation'; @@ -61,9 +60,9 @@ const PatternAssembler = ( { navigation, flow, stepName, - noticeList, noticeOperations, -}: StepProps & withNotices.Props ) => { + noticeUI, +}: StepProps & NoticesProps ) => { const translate = useTranslate(); const navigator = useNavigator(); const [ sectionPosition, setSectionPosition ] = useState< number | null >( null ); @@ -212,11 +211,6 @@ const PatternAssembler = ( { } ); }; - const showNotice = ( action: string, pattern: Pattern ) => { - noticeOperations.removeNotice( action ); - noticeOperations.createNotice( { id: action, content: getNoticeContent( action, pattern ) } ); - }; - const getDesign = () => ( { ...selectedDesign, @@ -238,12 +232,12 @@ const PatternAssembler = ( { updateActivePatternPosition( -1 ); if ( pattern ) { if ( header ) { - showNotice( 'replace', pattern ); + noticeOperations.showPatternReplacedNotice( pattern ); } else { - showNotice( 'add', pattern ); + noticeOperations.showPatternInsertedNotice( pattern ); } } else if ( header ) { - showNotice( 'remove', header ); + noticeOperations.showPatternRemovedNotice( header ); } }; @@ -257,12 +251,12 @@ const PatternAssembler = ( { activateFooterPosition( !! pattern ); if ( pattern ) { if ( footer ) { - showNotice( 'replace', pattern ); + noticeOperations.showPatternReplacedNotice( pattern ); } else { - showNotice( 'add', pattern ); + noticeOperations.showPatternInsertedNotice( pattern ); } } else if ( footer ) { - showNotice( 'remove', footer ); + noticeOperations.showPatternRemovedNotice( footer ); } }; @@ -277,7 +271,7 @@ const PatternAssembler = ( { ...sections.slice( sectionPosition + 1 ), ] ); updateActivePatternPosition( sectionPosition ); - showNotice( 'replace', pattern ); + noticeOperations.showPatternReplacedNotice( pattern ); } }; @@ -290,11 +284,11 @@ const PatternAssembler = ( { }, ] ); updateActivePatternPosition( sections.length ); - showNotice( 'add', pattern ); + noticeOperations.showPatternInsertedNotice( pattern ); }; const deleteSection = ( position: number ) => { - showNotice( 'remove', sections[ position ] ); + noticeOperations.showPatternRemovedNotice( sections[ position ] ); setSections( [ ...sections.slice( 0, position ), ...sections.slice( position + 1 ) ] ); updateActivePatternPosition( position ); }; @@ -565,7 +559,7 @@ const PatternAssembler = ( { ref={ wrapperRef } tabIndex={ -1 } > - + { noticeUI }
( +const PatternAssemblerStep = ( props: StepProps & NoticesProps ) => ( diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx index 9f9a20b11830a2..e7174620606c60 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx @@ -1,58 +1,101 @@ -import { SnackbarList, withNotices } from '@wordpress/components'; +import { NoticeList, SnackbarList } from '@wordpress/components'; +import { createHigherOrderComponent } from '@wordpress/compose'; import i18n from 'i18n-calypso'; -import { ReactNode, useEffect, useRef } from 'react'; +import { ComponentType, ReactNode, ReactChild, useMemo, useState, useCallback } from 'react'; import type { Pattern } from '../types'; import './notices.scss'; const NOTICE_TIMEOUT = 5000; -const Notices = ( { - noticeList, - noticeOperations, -}: Pick< withNotices.Props, 'noticeList' | 'noticeOperations' > ) => { - const onRemoveNotice = ( id: string ) => { - noticeOperations.removeNotice( id ); - }; - - const timersByContentRef = useRef< Map< ReactNode, ReturnType< typeof setTimeout > > >( - new Map() - ); - useEffect( () => { - timersByContentRef.current.forEach( ( timer, content ) => { - if ( ! noticeList.some( ( notice ) => notice.content === content ) ) { - clearTimeout( timer ); - timersByContentRef.current.delete( content ); - } - } ); - noticeList.forEach( ( notice ) => { - if ( ! timersByContentRef.current.has( notice.content ) ) { - timersByContentRef.current.set( - notice.content, - setTimeout( () => { - noticeOperations.removeNotice( notice.id ); - }, NOTICE_TIMEOUT ) - ); - } - } ); - }, [ noticeList, noticeOperations ] ); - - return ; +type Notice = NoticeList.Notice & { + timer?: ReturnType< typeof setTimeout >; }; -export const getNoticeContent = ( action: string, pattern: Pattern ) => { - const actions: { [ key: string ]: any } = { - add: i18n.translate( 'Block pattern "%(patternName)s" inserted.', { - args: { patternName: pattern.title }, - } ), - replace: i18n.translate( 'Block pattern "%(patternName)s" replaced.', { - args: { patternName: pattern.title }, - } ), - remove: i18n.translate( 'Block pattern "%(patternName)s" removed.', { - args: { patternName: pattern.title }, - } ), - }; - - return actions[ action ]; -}; +interface NoticeOperationsProps { + showPatternInsertedNotice: ( pattern: Pattern ) => void; + showPatternReplacedNotice: ( pattern: Pattern ) => void; + showPatternRemovedNotice: ( pattern: Pattern ) => void; +} + +export interface NoticesProps { + noticeOperations: NoticeOperationsProps; + noticeUI: ReactNode; +} + +const withNotices = createHigherOrderComponent( + < OuterProps, >( InnerComponent: ComponentType< OuterProps > ) => { + return ( props: OuterProps & NoticesProps ) => { + const [ noticeList, setNoticeList ] = useState< Notice[] >( [] ); + + const removeNotice = useCallback( + ( id: string ) => { + setNoticeList( ( current ) => current.filter( ( notice ) => notice.id !== id ) ); + }, + [ setNoticeList ] + ); + + const noticeOperations: NoticeOperationsProps = useMemo( () => { + const createNotice = ( id: string, content: ReactChild ) => { + const existingNoticeWithSameId = noticeList.find( ( notice ) => notice.id === id ); + if ( existingNoticeWithSameId?.timer ) { + clearTimeout( existingNoticeWithSameId.timer ); + delete existingNoticeWithSameId.timer; + } + + const newNotice = { + id, + content, + timer: setTimeout( () => { + removeNotice( id ); + }, NOTICE_TIMEOUT ), + }; + + setNoticeList( ( current ) => [ + ...current.filter( ( notice ) => notice.id !== id ), + newNotice, + ] ); + }; + + return { + showPatternInsertedNotice: ( pattern: Pattern ) => { + createNotice( + 'pattern-inserted', + i18n.translate( 'Block pattern "%(patternName)s" inserted.', { + args: { patternName: pattern.title }, + } ) + ); + }, + showPatternReplacedNotice: ( pattern: Pattern ) => { + createNotice( + 'pattern-replaced', + i18n.translate( 'Block pattern "%(patternName)s" replaced.', { + args: { patternName: pattern.title }, + } ) + ); + }, + showPatternRemovedNotice: ( pattern: Pattern ) => { + createNotice( + 'pattern-removed', + i18n.translate( 'Block pattern "%(patternName)s" removed.', { + args: { patternName: pattern.title }, + } ) + ); + }, + }; + }, [ noticeList, removeNotice ] ); + + const noticeUI = ; + + const propsWithNotices = { + ...props, + noticeOperations, + noticeUI, + }; + + return ; + }; + }, + 'withNotices' +); -export default Notices; +export default withNotices; From 0599a7837778497af33911e1ef0852e51a2ce949 Mon Sep 17 00:00:00 2001 From: Ashar Fuadi Date: Wed, 21 Jun 2023 15:36:12 +0700 Subject: [PATCH 4/4] Remove memoizations --- .../pattern-assembler/notices/notices.tsx | 101 +++++++++--------- 1 file changed, 48 insertions(+), 53 deletions(-) diff --git a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx index e7174620606c60..a3197993942501 100644 --- a/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx +++ b/client/landing/stepper/declarative-flow/internals/steps-repository/pattern-assembler/notices/notices.tsx @@ -1,7 +1,7 @@ import { NoticeList, SnackbarList } from '@wordpress/components'; import { createHigherOrderComponent } from '@wordpress/compose'; import i18n from 'i18n-calypso'; -import { ComponentType, ReactNode, ReactChild, useMemo, useState, useCallback } from 'react'; +import { ComponentType, ReactNode, ReactChild, useState } from 'react'; import type { Pattern } from '../types'; import './notices.scss'; @@ -27,62 +27,57 @@ const withNotices = createHigherOrderComponent( return ( props: OuterProps & NoticesProps ) => { const [ noticeList, setNoticeList ] = useState< Notice[] >( [] ); - const removeNotice = useCallback( - ( id: string ) => { - setNoticeList( ( current ) => current.filter( ( notice ) => notice.id !== id ) ); - }, - [ setNoticeList ] - ); - - const noticeOperations: NoticeOperationsProps = useMemo( () => { - const createNotice = ( id: string, content: ReactChild ) => { - const existingNoticeWithSameId = noticeList.find( ( notice ) => notice.id === id ); - if ( existingNoticeWithSameId?.timer ) { - clearTimeout( existingNoticeWithSameId.timer ); - delete existingNoticeWithSameId.timer; - } + const removeNotice = ( id: string ) => { + setNoticeList( ( current ) => current.filter( ( notice ) => notice.id !== id ) ); + }; - const newNotice = { - id, - content, - timer: setTimeout( () => { - removeNotice( id ); - }, NOTICE_TIMEOUT ), - }; + const createNotice = ( id: string, content: ReactChild ) => { + const existingNoticeWithSameId = noticeList.find( ( notice ) => notice.id === id ); + if ( existingNoticeWithSameId?.timer ) { + clearTimeout( existingNoticeWithSameId.timer ); + delete existingNoticeWithSameId.timer; + } - setNoticeList( ( current ) => [ - ...current.filter( ( notice ) => notice.id !== id ), - newNotice, - ] ); + const newNotice = { + id, + content, + timer: setTimeout( () => { + removeNotice( id ); + }, NOTICE_TIMEOUT ), }; - return { - showPatternInsertedNotice: ( pattern: Pattern ) => { - createNotice( - 'pattern-inserted', - i18n.translate( 'Block pattern "%(patternName)s" inserted.', { - args: { patternName: pattern.title }, - } ) - ); - }, - showPatternReplacedNotice: ( pattern: Pattern ) => { - createNotice( - 'pattern-replaced', - i18n.translate( 'Block pattern "%(patternName)s" replaced.', { - args: { patternName: pattern.title }, - } ) - ); - }, - showPatternRemovedNotice: ( pattern: Pattern ) => { - createNotice( - 'pattern-removed', - i18n.translate( 'Block pattern "%(patternName)s" removed.', { - args: { patternName: pattern.title }, - } ) - ); - }, - }; - }, [ noticeList, removeNotice ] ); + setNoticeList( ( current ) => [ + ...current.filter( ( notice ) => notice.id !== id ), + newNotice, + ] ); + }; + + const noticeOperations: NoticeOperationsProps = { + showPatternInsertedNotice: ( pattern: Pattern ) => { + createNotice( + 'pattern-inserted', + i18n.translate( 'Block pattern "%(patternName)s" inserted.', { + args: { patternName: pattern.title }, + } ) + ); + }, + showPatternReplacedNotice: ( pattern: Pattern ) => { + createNotice( + 'pattern-replaced', + i18n.translate( 'Block pattern "%(patternName)s" replaced.', { + args: { patternName: pattern.title }, + } ) + ); + }, + showPatternRemovedNotice: ( pattern: Pattern ) => { + createNotice( + 'pattern-removed', + i18n.translate( 'Block pattern "%(patternName)s" removed.', { + args: { patternName: pattern.title }, + } ) + ); + }, + }; const noticeUI = ;