From 5ab6083f2bda787144be15e99f4fc04f2dfb2036 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 21 Sep 2023 10:41:03 +1200 Subject: [PATCH 1/5] Return a promise from the category creation method so we can ensure it is finished before saving the pattern --- .../src/components/category-selector.js | 25 +++++++++++++------ .../src/components/create-pattern-modal.js | 22 ++++++++++++++-- 2 files changed, 38 insertions(+), 9 deletions(-) diff --git a/packages/patterns/src/components/category-selector.js b/packages/patterns/src/components/category-selector.js index c9305806c7e13f..1fca4b21349f23 100644 --- a/packages/patterns/src/components/category-selector.js +++ b/packages/patterns/src/components/category-selector.js @@ -29,7 +29,10 @@ const DEFAULT_QUERY = { }; const slug = 'wp_pattern_category'; -export default function CategorySelector( { onCategorySelection } ) { +export default function CategorySelector( { + onCategorySelection, + setCategorySaving, +} ) { const [ values, setValues ] = useState( [] ); const [ search, setSearch ] = useState( '' ); const debouncedSearch = useDebounce( setSearch, 500 ); @@ -92,13 +95,21 @@ export default function CategorySelector( { onCategorySelection } ) { setValues( uniqueTerms ); - Promise.all( - uniqueTerms.map( ( termName ) => - findOrCreateTerm( { name: termName } ) - ) - ).then( ( newTerms ) => { - onCategorySelection( newTerms ); + // If the user clicks the create pattern modal button directly after entering + // a category we need to return a promise so the pattern doesn't save before + // the save of the categories is completed. + const categorySaving = new Promise( function ( resolve ) { + Promise.all( + uniqueTerms.map( ( termName ) => + findOrCreateTerm( { name: termName } ) + ) + ).then( ( newTerms ) => { + setCategorySaving(); + onCategorySelection( newTerms ); + resolve( newTerms ); + } ); } ); + setCategorySaving( categorySaving ); } return ( diff --git a/packages/patterns/src/components/create-pattern-modal.js b/packages/patterns/src/components/create-pattern-modal.js index 189004b6a046b2..817dc1713fd3e0 100644 --- a/packages/patterns/src/components/create-pattern-modal.js +++ b/packages/patterns/src/components/create-pattern-modal.js @@ -36,16 +36,18 @@ export default function CreatePatternModal( { const [ syncType, setSyncType ] = useState( PATTERN_SYNC_TYPES.full ); const [ categories, setCategories ] = useState( [] ); const [ title, setTitle ] = useState( '' ); + const [ categorySaving, setCategorySaving ] = useState(); const { createPattern } = unlock( useDispatch( patternsStore ) ); const { createErrorNotice } = useDispatch( noticesStore ); - async function onCreate( patternTitle, sync ) { + + async function addPattern( patternTitle, sync, patternCategories ) { try { const newPattern = await createPattern( patternTitle, sync, typeof content === 'function' ? content() : content, - categories + patternCategories ); onSuccess( { pattern: newPattern, @@ -60,6 +62,21 @@ export default function CreatePatternModal( { } } + function onCreate( patternTitle, sync ) { + // Check that any onBlur save of the categories is completed + // before creating the pattern and closing the modal. + if ( categorySaving ) { + return categorySaving.then( ( newTerms ) => { + addPattern( + patternTitle, + sync, + newTerms.map( ( cat ) => cat.id ) + ); + } ); + } + addPattern( patternTitle, sync, categories ); + } + const handleCategorySelection = ( selectedCategories ) => { setCategories( selectedCategories.map( ( cat ) => cat.id ) ); }; @@ -91,6 +108,7 @@ export default function CreatePatternModal( { /> Date: Thu, 21 Sep 2023 10:54:40 +1200 Subject: [PATCH 2/5] Switch to an await instead of promise then() --- .../src/components/create-pattern-modal.js | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/packages/patterns/src/components/create-pattern-modal.js b/packages/patterns/src/components/create-pattern-modal.js index 817dc1713fd3e0..2702e4674cc145 100644 --- a/packages/patterns/src/components/create-pattern-modal.js +++ b/packages/patterns/src/components/create-pattern-modal.js @@ -62,17 +62,17 @@ export default function CreatePatternModal( { } } - function onCreate( patternTitle, sync ) { + async function onCreate( patternTitle, sync ) { // Check that any onBlur save of the categories is completed // before creating the pattern and closing the modal. if ( categorySaving ) { - return categorySaving.then( ( newTerms ) => { - addPattern( - patternTitle, - sync, - newTerms.map( ( cat ) => cat.id ) - ); - } ); + const newTerms = await categorySaving; + addPattern( + patternTitle, + sync, + newTerms.map( ( cat ) => cat.id ) + ); + return; } addPattern( patternTitle, sync, categories ); } From 7458095cf72eb728efd5d3203ba8d1eaea549320 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 21 Sep 2023 15:16:03 +1200 Subject: [PATCH 3/5] Remove redundant extra promise wrapper and disable button on saving --- .../src/components/category-selector.js | 22 +++++++++---------- .../src/components/create-pattern-modal.js | 19 +++++++++++----- 2 files changed, 24 insertions(+), 17 deletions(-) diff --git a/packages/patterns/src/components/category-selector.js b/packages/patterns/src/components/category-selector.js index 1fca4b21349f23..46d461b478a8a8 100644 --- a/packages/patterns/src/components/category-selector.js +++ b/packages/patterns/src/components/category-selector.js @@ -31,7 +31,7 @@ const slug = 'wp_pattern_category'; export default function CategorySelector( { onCategorySelection, - setCategorySaving, + setSaveCategoryPromise, } ) { const [ values, setValues ] = useState( [] ); const [ search, setSearch ] = useState( '' ); @@ -98,18 +98,16 @@ export default function CategorySelector( { // If the user clicks the create pattern modal button directly after entering // a category we need to return a promise so the pattern doesn't save before // the save of the categories is completed. - const categorySaving = new Promise( function ( resolve ) { - Promise.all( - uniqueTerms.map( ( termName ) => - findOrCreateTerm( { name: termName } ) - ) - ).then( ( newTerms ) => { - setCategorySaving(); - onCategorySelection( newTerms ); - resolve( newTerms ); - } ); + const categorySaving = Promise.all( + uniqueTerms.map( ( termName ) => + findOrCreateTerm( { name: termName } ) + ) + ).then( ( newTerms ) => { + setSaveCategoryPromise(); + onCategorySelection( newTerms ); + return newTerms; } ); - setCategorySaving( categorySaving ); + setSaveCategoryPromise( categorySaving ); } return ( diff --git a/packages/patterns/src/components/create-pattern-modal.js b/packages/patterns/src/components/create-pattern-modal.js index 2702e4674cc145..bad50c480e1b30 100644 --- a/packages/patterns/src/components/create-pattern-modal.js +++ b/packages/patterns/src/components/create-pattern-modal.js @@ -36,7 +36,8 @@ export default function CreatePatternModal( { const [ syncType, setSyncType ] = useState( PATTERN_SYNC_TYPES.full ); const [ categories, setCategories ] = useState( [] ); const [ title, setTitle ] = useState( '' ); - const [ categorySaving, setCategorySaving ] = useState(); + const [ saveCategoryPromise, setSaveCategoryPromise ] = useState(); + const [ isSaving, setIsSaving ] = useState(); const { createPattern } = unlock( useDispatch( patternsStore ) ); const { createErrorNotice } = useDispatch( noticesStore ); @@ -49,11 +50,13 @@ export default function CreatePatternModal( { typeof content === 'function' ? content() : content, patternCategories ); + setIsSaving( false ); onSuccess( { pattern: newPattern, categoryId: PATTERN_DEFAULT_CATEGORY, } ); } catch ( error ) { + setIsSaving( false ); createErrorNotice( error.message, { type: 'snackbar', id: 'convert-to-pattern-error', @@ -63,10 +66,11 @@ export default function CreatePatternModal( { } async function onCreate( patternTitle, sync ) { + setIsSaving( true ); // Check that any onBlur save of the categories is completed // before creating the pattern and closing the modal. - if ( categorySaving ) { - const newTerms = await categorySaving; + if ( saveCategoryPromise ) { + const newTerms = await saveCategoryPromise; addPattern( patternTitle, sync, @@ -108,7 +112,7 @@ export default function CreatePatternModal( { /> - From 5ed6eaa8a08b210017aca6412f73f1b6827878af Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 21 Sep 2023 15:20:48 +1200 Subject: [PATCH 4/5] Add is busy prop --- packages/patterns/src/components/create-pattern-modal.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/patterns/src/components/create-pattern-modal.js b/packages/patterns/src/components/create-pattern-modal.js index bad50c480e1b30..5704f537edb525 100644 --- a/packages/patterns/src/components/create-pattern-modal.js +++ b/packages/patterns/src/components/create-pattern-modal.js @@ -144,6 +144,7 @@ export default function CreatePatternModal( { type="submit" disabled={ isSaving } aria-disabled={ isSaving } + isBusy={ isSaving } > { __( 'Create' ) } From f0ee8990cb6843ad91042c6d4824496e7539c27c Mon Sep 17 00:00:00 2001 From: Kai Hao Date: Thu, 21 Sep 2023 15:09:26 +0800 Subject: [PATCH 5/5] Refactor to only create on save --- .../src/components/category-selector.js | 61 ++-------------- .../src/components/create-pattern-modal.js | 70 +++++++++++-------- 2 files changed, 48 insertions(+), 83 deletions(-) diff --git a/packages/patterns/src/components/category-selector.js b/packages/patterns/src/components/category-selector.js index 46d461b478a8a8..397d851d3886b9 100644 --- a/packages/patterns/src/components/category-selector.js +++ b/packages/patterns/src/components/category-selector.js @@ -4,7 +4,7 @@ import { __ } from '@wordpress/i18n'; import { useMemo, useState } from '@wordpress/element'; import { FormTokenField } from '@wordpress/components'; -import { useSelect, useDispatch } from '@wordpress/data'; +import { useSelect } from '@wordpress/data'; import { store as coreStore } from '@wordpress/core-data'; import { useDebounce } from '@wordpress/compose'; import { decodeEntities } from '@wordpress/html-entities'; @@ -13,13 +13,6 @@ const unescapeString = ( arg ) => { return decodeEntities( arg ); }; -const unescapeTerm = ( term ) => { - return { - ...term, - name: unescapeString( term.name ), - }; -}; - const EMPTY_ARRAY = []; const MAX_TERMS_SUGGESTIONS = 20; const DEFAULT_QUERY = { @@ -27,16 +20,11 @@ const DEFAULT_QUERY = { _fields: 'id,name', context: 'view', }; -const slug = 'wp_pattern_category'; +export const CATEGORY_SLUG = 'wp_pattern_category'; -export default function CategorySelector( { - onCategorySelection, - setSaveCategoryPromise, -} ) { - const [ values, setValues ] = useState( [] ); +export default function CategorySelector( { values, onChange } ) { const [ search, setSearch ] = useState( '' ); const debouncedSearch = useDebounce( setSearch, 500 ); - const { invalidateResolution } = useDispatch( coreStore ); const { searchResults } = useSelect( ( select ) => { @@ -44,7 +32,7 @@ export default function CategorySelector( { return { searchResults: !! search - ? getEntityRecords( 'taxonomy', slug, { + ? getEntityRecords( 'taxonomy', CATEGORY_SLUG, { ...DEFAULT_QUERY, search, } ) @@ -60,28 +48,7 @@ export default function CategorySelector( { ); }, [ searchResults ] ); - const { saveEntityRecord } = useDispatch( coreStore ); - - async function findOrCreateTerm( term ) { - try { - const newTerm = await saveEntityRecord( 'taxonomy', slug, term, { - throwOnError: true, - } ); - invalidateResolution( 'getUserPatternCategories' ); - return unescapeTerm( newTerm ); - } catch ( error ) { - if ( error.code !== 'term_exists' ) { - throw error; - } - - return { - id: error.data.term_id, - name: term.name, - }; - } - } - - function onChange( termNames ) { + function handleChange( termNames ) { const uniqueTerms = termNames.reduce( ( terms, newTerm ) => { if ( ! terms.some( @@ -93,21 +60,7 @@ export default function CategorySelector( { return terms; }, [] ); - setValues( uniqueTerms ); - - // If the user clicks the create pattern modal button directly after entering - // a category we need to return a promise so the pattern doesn't save before - // the save of the categories is completed. - const categorySaving = Promise.all( - uniqueTerms.map( ( termName ) => - findOrCreateTerm( { name: termName } ) - ) - ).then( ( newTerms ) => { - setSaveCategoryPromise(); - onCategorySelection( newTerms ); - return newTerms; - } ); - setSaveCategoryPromise( categorySaving ); + onChange( uniqueTerms ); } return ( @@ -116,7 +69,7 @@ export default function CategorySelector( { className="patterns-menu-items__convert-modal-categories" value={ values } suggestions={ suggestions } - onChange={ onChange } + onChange={ handleChange } onInputChange={ debouncedSearch } maxSuggestions={ MAX_TERMS_SUGGESTIONS } label={ __( 'Categories' ) } diff --git a/packages/patterns/src/components/create-pattern-modal.js b/packages/patterns/src/components/create-pattern-modal.js index 5704f537edb525..8dad206f3f1e66 100644 --- a/packages/patterns/src/components/create-pattern-modal.js +++ b/packages/patterns/src/components/create-pattern-modal.js @@ -13,6 +13,7 @@ import { __ } from '@wordpress/i18n'; import { useState } from '@wordpress/element'; import { useDispatch } from '@wordpress/data'; import { store as noticesStore } from '@wordpress/notices'; +import { store as coreStore } from '@wordpress/core-data'; /** * Internal dependencies @@ -23,7 +24,7 @@ import { PATTERN_DEFAULT_CATEGORY, PATTERN_SYNC_TYPES } from '../constants'; * Internal dependencies */ import { store as patternsStore } from '../store'; -import CategorySelector from './category-selector'; +import CategorySelector, { CATEGORY_SLUG } from './category-selector'; import { unlock } from '../lock-unlock'; export default function CreatePatternModal( { @@ -34,57 +35,70 @@ export default function CreatePatternModal( { className = 'patterns-menu-items__convert-modal', } ) { const [ syncType, setSyncType ] = useState( PATTERN_SYNC_TYPES.full ); - const [ categories, setCategories ] = useState( [] ); + const [ categoryTerms, setCategoryTerms ] = useState( [] ); const [ title, setTitle ] = useState( '' ); - const [ saveCategoryPromise, setSaveCategoryPromise ] = useState(); - const [ isSaving, setIsSaving ] = useState(); + const [ isSaving, setIsSaving ] = useState( false ); const { createPattern } = unlock( useDispatch( patternsStore ) ); - + const { saveEntityRecord, invalidateResolution } = useDispatch( coreStore ); const { createErrorNotice } = useDispatch( noticesStore ); - async function addPattern( patternTitle, sync, patternCategories ) { + async function onCreate( patternTitle, sync ) { + if ( isSaving ) return; + try { + setIsSaving( true ); + const categories = await Promise.all( + categoryTerms.map( ( termName ) => + findOrCreateTerm( termName ) + ) + ); + const newPattern = await createPattern( patternTitle, sync, typeof content === 'function' ? content() : content, - patternCategories + categories ); - setIsSaving( false ); onSuccess( { pattern: newPattern, categoryId: PATTERN_DEFAULT_CATEGORY, } ); } catch ( error ) { - setIsSaving( false ); createErrorNotice( error.message, { type: 'snackbar', id: 'convert-to-pattern-error', } ); onError(); + } finally { + setIsSaving( false ); + setCategoryTerms( [] ); + setTitle( '' ); } } - async function onCreate( patternTitle, sync ) { - setIsSaving( true ); - // Check that any onBlur save of the categories is completed - // before creating the pattern and closing the modal. - if ( saveCategoryPromise ) { - const newTerms = await saveCategoryPromise; - addPattern( - patternTitle, - sync, - newTerms.map( ( cat ) => cat.id ) + /** + * @param {string} term + * @return {Promise} The pattern category id. + */ + async function findOrCreateTerm( term ) { + try { + const newTerm = await saveEntityRecord( + 'taxonomy', + CATEGORY_SLUG, + { name: term }, + { throwOnError: true } ); - return; + invalidateResolution( 'getUserPatternCategories' ); + return newTerm.id; + } catch ( error ) { + if ( error.code !== 'term_exists' ) { + throw error; + } + + return error.data.term_id; } - addPattern( patternTitle, sync, categories ); } - const handleCategorySelection = ( selectedCategories ) => { - setCategories( selectedCategories.map( ( cat ) => cat.id ) ); - }; - return ( { event.preventDefault(); onCreate( title, syncType ); - setTitle( '' ); } } > @@ -111,8 +124,8 @@ export default function CreatePatternModal( { className="patterns-create-modal__name-input" />