Add quotes to font family names

[vcanales]

Checks if the font family name of a given font, either added through Google Fonts or Locally, ends with a number and if so, it adds single quotes to its name. Add single quotes to font family names to avoid errors related to trying to parse font names with special characters, white spaces, or numbers.

Also, check if the font family name of a font being uploaded includes non-alphanumeric characters and invalidates the form if so.

Currently, no validation errors are shown in the UI; I'll be making a note of this to add it in as a follow-up change.

Fixes: #468

[vcanales]

I've also added a couple of optimizations that I noticed while fixing this:

1. Refactor promise calls to use async/await syntax, for consistency's sake.
2. Avoid re renders by wrapping isFormValid with useCallback, and also move the definition of onFormDataChange inside the effect where it's used to avoid having to pass it as a dependency.

[mikachan]

This works in my testing for the font Exo 2, but I don't think it'll catch the names of the following fonts:

• Libre Barcode 128 Text
• Press Start 2P
• Rock 3D
• Rubik 80s Fade

I've left a longer comment about this below, but I'm wondering if it may be best practice to always wrap font names in quotes, as long as the name isn't a generic fallback name (which they're unlikely to be, as they're custom fonts).

Refactor promise calls to use async/await syntax
Avoid re renders by wrapping isFormValid with useCallback,

Nice!

Definitely not a blocker for this PR but it would be great if we could write some tests, especially for things like the utils functions which are hopefully easier to cover in isolation. I know we've not historically done this in this repo - the list of example fonts above made me think that they'd make great test cases.

[vcanales]

@mikachan You're correct in that those cases should also be considered. Here's a few examples:

// Normal
> new FontFace('Example1', {});
< FontFace { family: "Example1", style: "normal", weight: "normal", stretch: "normal", unicodeRange: "U+0-10FFFF", variant: "normal", featureSettings: "normal", variationSettings: "normal", display: "auto", ascentOverride: "normal" }

// Notice the empty family name:
> new FontFace('Example 2', {});
< FontFace { family: "", style: "normal", weight: "normal", stretch: "normal", unicodeRange: "U+0-10FFFF", variant: "normal", featureSettings: "normal", variationSettings: "normal", display: "auto", ascentOverride: "normal" }

// Same, empty family name:
new FontFace('Rubik 80s Fade', {});
FontFace { family: "", style: "normal", weight: "normal", stretch: "normal", unicodeRange: "U+0-10FFFF", variant: "normal", featureSettings: "normal", variationSettings: "normal", display: "auto", ascentOverride: "normal" }

For some reason, fonts that have white spaces and numbers in their name didn't output errors, but they're just as wrong. I'll have addQuotesToName just add quotes and trim names without conditions. I can't seem to find any downsides to doing this.

[vcanales]

Definitely not a blocker for this PR but it would be great if we could write some tests, especially for things like the utils functions which are hopefully easier to cover in isolation. I know we've not historically done this in this repo - the list of example fonts above made me think that they'd make great test cases.

I don't think we even have the tools for this right now, so I'll add it to my to-do for this week. I agree that this change in particular is very well fit for unit tests.

[mikachan]

We should add the new addQuotesToName function to demoStyle too (around line 56 of local-fonts/index.js), something like this:

		const style = {
			fontFamily: addQuotesToName( formData.name ),
			fontWeight: formData.weight,
			fontStyle: formData.style,
		};

This allows the inline font demo on the local fonts page to apply the font family correctly.

[vcanales]

@mikachan I ended up adding tests here; it was simpler than expected. I added @wordpress/wp-scripts as a dependency now, which undoes #467.

[mikachan]

I ended up adding tests here; it was simpler than expected.

Awesome! This is a great start. With this we can start adding tests in other areas too!

[mikachan]

Thanks for working on all of this! I think we should bring this in.