Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Font Library: Make notices more consistent #58180

Merged
merged 30 commits into from
Jan 26, 2024
Merged

Font Library: Make notices more consistent #58180

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
c8072d4
Font Library: add wp_font_face post type and scaffold font face REST …
creativecoder Jan 9, 2024
61da35c
Font Library: create font faces through the REST API (#57702)
creativecoder Jan 11, 2024
42565a3
Refactor Font Family Controller (#57785)
creativecoder Jan 12, 2024
afacdf2
Font Family and Font Face REST API endpoints: better data handling an…
creativecoder Jan 15, 2024
7c82cb9
Font Families REST API endpoint: ensure unique font family slugs (#57…
creativecoder Jan 16, 2024
f84cc6d
Font Library: delete child font faces and font assets when deleting p…
creativecoder Jan 16, 2024
492a3ee
Font Library: refactor client side install functions to work with rev…
jffng Jan 17, 2024
9ebc25f
Cleanup/font library view error handling (#57926)
pbking Jan 17, 2024
8a5efd4
Font Faces endpoint: prevent creating font faces with duplicate setti…
creativecoder Jan 17, 2024
3a739bc
Font Library: Update uninstall/delete on client side (#57932)
mikachan Jan 18, 2024
c6e0fbb
Update packages/edit-site/src/components/global-styles/font-library-m…
mikachan Jan 18, 2024
74be330
Merge branch 'trunk' into try/font-library-refactor
mikachan Jan 18, 2024
a666bb5
Font Library: address JS feedback in #57688 (#57961)
mikachan Jan 18, 2024
1aa3b0c
Add missing dep in font-demo
mikachan Jan 18, 2024
0d92b6f
Move notice to top of local-fonts panel
mikachan Jan 18, 2024
2502ebf
Add container around spinner
mikachan Jan 18, 2024
552842b
Move notice to TabPanelLayout
mikachan Jan 19, 2024
cc8e4f8
Remove spacer from LocalFonts
mikachan Jan 19, 2024
c4bef05
Move notice and setNotice state to context.js
mikachan Jan 19, 2024
524972f
Move spacer outside of notice container
mikachan Jan 19, 2024
e9d29d9
Merge branch 'trunk' into font-library/ui-tweaks
mikachan Jan 24, 2024
049e5a5
Rename LocalFonts to UploadFonts
mikachan Jan 24, 2024
c323c44
Make notices dismissible
mikachan Jan 24, 2024
92fea20
Reset notices onRemove
mikachan Jan 24, 2024
2bf44c2
Reset notice when new tab is selected
mikachan Jan 24, 2024
e7fb1e8
Reset notice on each action
mikachan Jan 25, 2024
7e94ee8
Merge branch 'trunk' into font-library/update-notices
mikachan Jan 25, 2024
4750131
Merge branch 'trunk' into font-library/update-notices
mikachan Jan 25, 2024
9705a48
Fix notice re-render on fetchFontCollection
mikachan Jan 25, 2024
ccf7f40
Move notices below font collection title and description
mikachan Jan 25, 2024
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
14 changes: 14 additions & 0 deletions packages/edit-site/src/components/global-styles/font-library-modal/context.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,21 @@ function FontLibraryProvider( { children } ) {

const [ isInstalling, setIsInstalling ] = useState( false );
const [ refreshKey, setRefreshKey ] = useState( 0 );
const [ notice, setNotice ] = useState( null );

const refreshLibrary = () => {
setRefreshKey( Date.now() );
};

// Reset notice on dismiss.
useEffect( () => {
if ( notice ) {
notice.onRemove = () => {
setNotice( null );
};
}
}, [ notice, setNotice ] );

const {
records: libraryPosts = [],
isResolving: isResolvingLibrary,
Expand Down Expand Up @@ -134,6 +144,8 @@ function FontLibraryProvider( { children } ) {
}, [ modalTabOpen ] );

const handleSetLibraryFontSelected = ( font ) => {
setNotice( null );

// If font is null, reset the selected font
if ( ! font ) {
setLibraryFontSelected( null );
Expand Down Expand Up @@ -471,6 +483,8 @@ function FontLibraryProvider( { children } ) {
modalTabOpen,
toggleModal,
refreshLibrary,
notice,
setNotice,
saveFontFamilies,
fontFamiliesHasChanges,
isResolvingLibrary,
Expand Down
48 changes: 12 additions & 36 deletions packages/edit-site/src/components/global-styles/font-library-modal/font-collection.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import {
FlexItem,
Flex,
Button,
Notice,
} from '@wordpress/components';
import { debounce } from '@wordpress/compose';
import { __ } from '@wordpress/i18n';
Expand Down Expand Up @@ -47,14 +46,13 @@ function FontCollection( { slug } ) {
);
};

const [ notice, setNotice ] = useState( null );
const [ selectedFont, setSelectedFont ] = useState( null );
const [ fontsToInstall, setFontsToInstall ] = useState( [] );
const [ filters, setFilters ] = useState( {} );
const [ renderConfirmDialog, setRenderConfirmDialog ] = useState(
requiresPermission && ! getGoogleFontsPermissionFromStorage()
);
const { collections, getFontCollection, installFont } =
const { collections, getFontCollection, installFont, notice, setNotice } =
useContext( FontLibraryContext );
const selectedCollection = collections.find(
( collection ) => collection.slug === slug
Expand All @@ -77,36 +75,27 @@ function FontCollection( { slug } ) {
await getFontCollection( slug );
resetFilters();
} catch ( e ) {
setNotice( {
type: 'error',
message: e?.message,
duration: 0, // Don't auto-hide.
} );
if ( ! notice ) {
Copy link
Contributor

@pbking pbking Jan 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Do we have to check for notice? I think we could remove this line.

Copy link
Member Author

@mikachan mikachan Jan 26, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I added this as the fetchFontCollection function keeps re-setting the notice on failure if there is an error with the font collection (e.g. something wrong with the JSON file), and it was causing an infinite loop of setting the error notice, I think because setNotice is a dependency of the useEffect where this function is called, so the notice was causing a re-render. This line fixed the issue (original discussion here). (This is happening on trunk, so I guess this isn't 100% related to this PR.)

setNotice( {
type: 'error',
message: e?.message,
} );
}
}
};
fetchFontCollection();
}, [ slug, getFontCollection ] );
}, [ slug, getFontCollection, setNotice, notice ] );

useEffect( () => {
setSelectedFont( null );
setNotice( null );
}, [ slug ] );
}, [ slug, setNotice ] );

useEffect( () => {
// If the selected fonts change, reset the selected fonts to install
setFontsToInstall( [] );
}, [ selectedFont ] );

// Reset notice after 5 seconds
useEffect( () => {
if ( notice && notice?.duration !== 0 ) {
const timeout = setTimeout( () => {
setNotice( null );
}, notice.duration ?? 5000 );
return () => clearTimeout( timeout );
}
}, [ notice ] );

const collectionFonts = useMemo(
() => selectedCollection?.font_families ?? [],
[ selectedCollection ]
Expand Down Expand Up @@ -154,6 +143,8 @@ function FontCollection( { slug } ) {
};

const handleInstall = async () => {
setNotice( null );

const fontFamily = fontsToInstall[ 0 ];

try {
Expand Down Expand Up @@ -205,6 +196,7 @@ function FontCollection( { slug } ) {
? selectedCollection.description
: __( 'Select font variants to install.' )
}
notice={ notice }
handleBack={ !! selectedFont && handleUnselectFont }
footer={
fontsToInstall.length > 0 && (
Expand All @@ -219,22 +211,6 @@ function FontCollection( { slug } ) {
</>
) }

{ notice && (
<>
<FlexItem>
<Spacer margin={ 2 } />
<Notice
isDismissible={ false }
status={ notice.type }
className="font-library-modal__font-collection__notice"
>
{ notice.message }
</Notice>
</FlexItem>
<Spacer margin={ 2 } />
</>
) }

{ ! renderConfirmDialog && ! selectedFont && (
<Flex>
<FlexItem>
Expand Down
2 changes: 1 addition & 1 deletion packages/edit-site/src/components/global-styles/font-library-modal/font-demo.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ function FontFaceDemo( { customPreviewUrl, fontFace, text, style = {} } ) {
}
};
loadAsset();
}, [ fontFace, isIntersecting, loadFontFaceAsset ] );
}, [ fontFace, isIntersecting, loadFontFaceAsset, isPreviewImage ] );
Copy link
Member Author

Choose a reason for hiding this comment

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

This is unrelated, I just noticed it needed updating.


return (
<div ref={ ref }>
Expand Down
9 changes: 7 additions & 2 deletions packages/edit-site/src/components/global-styles/font-library-modal/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,13 +43,18 @@ function FontLibraryModal( {
onRequestClose,
initialTabId = 'installed-fonts',
} ) {
const { collections } = useContext( FontLibraryContext );
const { collections, setNotice } = useContext( FontLibraryContext );

const tabs = [
...DEFAULT_TABS,
...tabsFromCollections( collections || [] ),
];

// Reset notice when new tab is selected.
const onSelect = () => {
setNotice( null );
};

return (
<Modal
title={ __( 'Fonts' ) }
Expand All @@ -58,7 +63,7 @@ function FontLibraryModal( {
className="font-library-modal"
>
<div className="font-library-modal__tabs">
<Tabs initialTabId={ initialTabId }>
<Tabs initialTabId={ initialTabId } onSelect={ onSelect }>
<Tabs.TabList>
{ tabs.map( ( { id, title } ) => (
<Tabs.Tab key={ id } tabId={ id }>
Expand Down
43 changes: 12 additions & 31 deletions packages/edit-site/src/components/global-styles/font-library-modal/installed-fonts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import {
__experimentalSpacer as Spacer,
Button,
Spinner,
Notice,
FlexItem,
} from '@wordpress/components';

Expand All @@ -34,6 +33,8 @@ function InstalledFonts() {
refreshLibrary,
uninstallFontFamily,
isResolvingLibrary,
notice,
setNotice,
} = useContext( FontLibraryContext );
const [ isConfirmDeleteOpen, setIsConfirmDeleteOpen ] = useState( false );

Expand All @@ -45,9 +46,9 @@ function InstalledFonts() {
handleSetLibraryFontSelected( font );
};

const [ notice, setNotice ] = useState( null );

const handleConfirmUninstall = async () => {
setNotice( null );

try {
await uninstallFontFamily( libraryFontSelected );
setNotice( {
Expand Down Expand Up @@ -91,20 +92,11 @@ function InstalledFonts() {
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [] );

// Reset notice after 5 seconds
useEffect( () => {
if ( notice ) {
const timeout = setTimeout( () => {
setNotice( null );
}, 5000 );
return () => clearTimeout( timeout );
}
}, [ notice ] );

return (
<TabPanelLayout
title={ libraryFontSelected?.name || '' }
description={ tabDescription }
notice={ notice }
handleBack={ !! libraryFontSelected && handleUnselectFont }
footer={
<Footer
Expand All @@ -120,28 +112,17 @@ function InstalledFonts() {
handleCancelUninstall={ handleCancelUninstall }
/>

{ notice && (
<>
<FlexItem>
<Spacer margin={ 2 } />
<Notice
isDismissible={ false }
status={ notice.type }
className="font-library-modal__font-collection__notice"
>
{ notice.message }
</Notice>
</FlexItem>
<Spacer margin={ 4 } />
</>
) }

{ ! libraryFontSelected && (
<>
{ isResolvingLibrary && <Spinner /> }
{ isResolvingLibrary && (
<FlexItem>
<Spacer margin={ 2 } />
<Spinner />
<Spacer margin={ 2 } />
</FlexItem>
) }
{ baseCustomFonts.length > 0 && (
<>
<Spacer margin={ 2 } />
<FontsGrid>
{ baseCustomFonts.map( ( font ) => (
<LibraryFontCard
Expand Down
Loading
Loading