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

Changed installFont to installFonts so that multiple font families can be installed at once #59451

Merged
merged 3 commits into from
Mar 1, 2024
Merged

Changed installFont to installFonts so that multiple font families can be installed at once #59451

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
3b30695
Changed installFont to installFonts so that multiple font families ca…
pbking Feb 29, 2024
c218998
Fix linter errors
mikachan Feb 29, 2024
e469b40
Fixed async loop used to install font families
pbking Feb 29, 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
185 changes: 88 additions & 97 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 @@ -200,120 +200,115 @@ function FontLibraryProvider( { children } ) {
return getActivatedFontsOutline( source )[ slug ] || [];
};

async function installFont( fontFamilyToInstall ) {
async function installFonts( fontFamiliesToInstall ) {
setIsInstalling( true );
let isANewFontFamily = false;
try {
// Get the font family if it already exists.
let installedFontFamily = await fetchGetFontFamilyBySlug(
fontFamilyToInstall.slug
);
const fontFamiliesToActivate = [];
let installationErrors = [];

for ( const fontFamilyToInstall of fontFamiliesToInstall ) {
let isANewFontFamily = false;

// Otherwise create it.
if ( ! installedFontFamily ) {
isANewFontFamily = true;
// Prepare font family form data to install.
installedFontFamily = await fetchInstallFontFamily(
makeFontFamilyFormData( fontFamilyToInstall )
// Get the font family if it already exists.
let installedFontFamily = await fetchGetFontFamilyBySlug(
fontFamilyToInstall.slug
);
}

// Collect font faces that have already been installed (to be activated later)
const alreadyInstalledFontFaces =
installedFontFamily.fontFace && fontFamilyToInstall.fontFace
? installedFontFamily.fontFace.filter(
// Otherwise create it.
if ( ! installedFontFamily ) {
isANewFontFamily = true;
// Prepare font family form data to install.
installedFontFamily = await fetchInstallFontFamily(
makeFontFamilyFormData( fontFamilyToInstall )
);
}

// Collect font faces that have already been installed (to be activated later)
const alreadyInstalledFontFaces =
installedFontFamily.fontFace && fontFamilyToInstall.fontFace
? installedFontFamily.fontFace.filter(
( fontFaceToInstall ) =>
checkFontFaceInstalled(
fontFaceToInstall,
fontFamilyToInstall.fontFace
)
)
: [];

// Filter out Font Faces that have already been installed (so that they are not re-installed)
if (
installedFontFamily.fontFace &&
fontFamilyToInstall.fontFace
) {
fontFamilyToInstall.fontFace =
fontFamilyToInstall.fontFace.filter(
( fontFaceToInstall ) =>
checkFontFaceInstalled(
! checkFontFaceInstalled(
fontFaceToInstall,
fontFamilyToInstall.fontFace
installedFontFamily.fontFace
)
)
: [];

// Filter out Font Faces that have already been installed (so that they are not re-installed)
if (
installedFontFamily.fontFace &&
fontFamilyToInstall.fontFace
) {
fontFamilyToInstall.fontFace =
fontFamilyToInstall.fontFace.filter(
( fontFaceToInstall ) =>
! checkFontFaceInstalled(
fontFaceToInstall,
installedFontFamily.fontFace
)
);
}

// Install the fonts (upload the font files to the server and create the post in the database).
let sucessfullyInstalledFontFaces = [];
let unsucessfullyInstalledFontFaces = [];
if ( fontFamilyToInstall?.fontFace?.length > 0 ) {
const response = await batchInstallFontFaces(
installedFontFamily.id,
makeFontFacesFormData( fontFamilyToInstall )
);
sucessfullyInstalledFontFaces = response?.successes;
unsucessfullyInstalledFontFaces = response?.errors;
}
);
}

const detailedErrorMessage = unsucessfullyInstalledFontFaces.reduce(
( errorMessageCollection, error ) => {
return `${ errorMessageCollection } ${ error.message }`;
},
''
);
// Install the fonts (upload the font files to the server and create the post in the database).
let sucessfullyInstalledFontFaces = [];
let unsucessfullyInstalledFontFaces = [];
if ( fontFamilyToInstall?.fontFace?.length > 0 ) {
const response = await batchInstallFontFaces(
installedFontFamily.id,
makeFontFacesFormData( fontFamilyToInstall )
);
sucessfullyInstalledFontFaces = response?.successes;
unsucessfullyInstalledFontFaces = response?.errors;
}

// If there were no successes and nothing already installed then we don't need to activate anything and can bounce now.
if (
fontFamilyToInstall?.fontFace?.length > 0 &&
sucessfullyInstalledFontFaces.length === 0 &&
alreadyInstalledFontFaces.length === 0
) {
if ( isANewFontFamily ) {
// Use the sucessfully installed font faces
// As well as any font faces that were already installed (those will be activated)
if (
sucessfullyInstalledFontFaces?.length > 0 ||
alreadyInstalledFontFaces?.length > 0
) {
fontFamilyToInstall.fontFace = [
...sucessfullyInstalledFontFaces,
...alreadyInstalledFontFaces,
];
fontFamiliesToActivate.push( fontFamilyToInstall );
} else if ( isANewFontFamily ) {
// If the font family is new, delete it to avoid having font families without font faces.
await fetchUninstallFontFamily( installedFontFamily.id );
}

throw new Error(
sprintf(
/* translators: %s: Specific error message returned from server. */
__( 'No font faces were installed. %s' ),
detailedErrorMessage
)
installationErrors = installationErrors.concat(
unsucessfullyInstalledFontFaces
);
}

// Use the sucessfully installed font faces
// As well as any font faces that were already installed (those will be activated)
if (
sucessfullyInstalledFontFaces?.length > 0 ||
alreadyInstalledFontFaces?.length > 0
) {
fontFamilyToInstall.fontFace = [
...sucessfullyInstalledFontFaces,
...alreadyInstalledFontFaces,
];
}

// Activate the font family (add the font family to the global styles).
activateCustomFontFamilies( [ fontFamilyToInstall ] );
if ( fontFamiliesToActivate.length > 0 ) {
// Activate the font family (add the font family to the global styles).
activateCustomFontFamilies( fontFamiliesToActivate );

// Save the global styles to the database.
saveSpecifiedEntityEdits( 'root', 'globalStyles', globalStylesId, [
'settings.typography.fontFamilies',
] );
// Save the global styles to the database.
await saveSpecifiedEntityEdits(
'root',
'globalStyles',
globalStylesId,
[ 'settings.typography.fontFamilies' ]
);

refreshLibrary();
refreshLibrary();
}

if ( unsucessfullyInstalledFontFaces.length > 0 ) {
if ( installationErrors.length > 0 ) {
throw new Error(
sprintf(
/* translators: %s: Specific error message returned from server. */
__(
'Some font faces were installed. There were some errors. %s'
),
detailedErrorMessage
__( 'There were some errors installing fonts. %s' ),
installationErrors.reduce(
( errorMessageCollection, error ) => {
return `${ errorMessageCollection } ${ error.message }`;
},
''
)
)
);
}
Expand Down Expand Up @@ -372,14 +367,10 @@ function FontLibraryProvider( { children } ) {

const activateCustomFontFamilies = ( fontsToAdd ) => {
// Merge the existing custom fonts with the new fonts.
const newCustomFonts = mergeFontFamilies(
fontFamilies?.custom,
fontsToAdd
);
// Activate the fonts by set the new custom fonts array.
setFontFamilies( {
...fontFamilies,
custom: newCustomFonts,
custom: mergeFontFamilies( fontFamilies?.custom, fontsToAdd ),
} );
// Add custom fonts to the browser.
fontsToAdd.forEach( ( font ) => {
Expand Down Expand Up @@ -464,7 +455,7 @@ function FontLibraryProvider( { children } ) {
isFontActivated,
getFontFacesActivated,
loadFontFaceAsset,
installFont,
installFonts,
uninstallFontFamily,
toggleActivateFont,
getAvailableFontsOutline,
Expand Down
4 changes: 2 additions & 2 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 @@ -77,7 +77,7 @@ function FontCollection( { slug } ) {
const {
collections,
getFontCollection,
installFont,
installFonts,
isInstalling,
notice,
setNotice,
Expand Down Expand Up @@ -214,7 +214,7 @@ function FontCollection( { slug } ) {
}

try {
await installFont( fontFamily );
await installFonts( [ fontFamily ] );
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
await installFonts( [ fontFamily ] );
await installFonts( fontFamily );

Only a small nitpick but I think I'd rather the installFonts function handled converting arguments to an array (e.g. checking if fontFamiliesToInstall is an array, if not then converting it), as I think it would slightly improve the readability here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm generally opposed to passing different types of arguments into a function. If we need to change how that value is passed to offer clarity I would rather do that (i.e. fontFamiliesToInstall = []; fontFamiliesToInstall.push( fontFamily ); installFonts( fontFamiliesToInstall );

Copy link
Member

Choose a reason for hiding this comment

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

Yeah I think that's a good alternative too 👍

setNotice( {
type: 'success',
message: __( 'Fonts were installed successfully.' ),
Expand Down
16 changes: 3 additions & 13 deletions packages/edit-site/src/components/global-styles/font-library-modal/upload-fonts.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,8 @@ import { unlock } from '../../../lock-unlock';
const { ProgressBar } = unlock( componentsPrivateApis );

function UploadFonts() {
const { installFont, notice, setNotice } = useContext( FontLibraryContext );
const { installFonts, notice, setNotice } =
useContext( FontLibraryContext );
const [ isUploading, setIsUploading ] = useState( false );

const handleDropZone = ( files ) => {
Expand Down Expand Up @@ -143,19 +144,8 @@ function UploadFonts() {
const handleInstall = async ( fontFaces ) => {
const fontFamilies = makeFamiliesFromFaces( fontFaces );

if ( fontFamilies.length > 1 ) {
setNotice( {
type: 'error',
message: __(
'Variants from only one font family can be uploaded at a time.'
),
} );
setIsUploading( false );
return;
}

try {
await installFont( fontFamilies[ 0 ] );
await installFonts( fontFamilies );
setNotice( {
type: 'success',
message: __( 'Fonts were installed successfully.' ),
Expand Down
Loading