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.

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

Font library: add support for detailed error messaging in API and UI #54777

Closed
madhusudhand opened this issue Sep 25, 2023 · 10 comments
Closed
Labels
Developer Experience Ideas about improving block and theme developer experience [Feature] Font Library [Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended

Comments

@madhusudhand
Copy link
Member

Description

The current implementation of API doesn't return detailed error when a font upload fails. Because of this reason, it is not possible to show appropriate error message in the UI.

Return detailed error messages from the API.

Step-by-step reproduction instructions

Try uploading a .otf file, and observe that API doesn't return any relevant error message or type.

Screenshots, screen recording, code snippet

No response

Environment info

  • Gutenberg trunk

Please confirm that you have searched existing issues in the repo.

Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

Yes

@madhusudhand madhusudhand added the [Type] Bug An existing feature does not function as intended label Sep 25, 2023
@properlypurple properlypurple added Developer Experience Ideas about improving block and theme developer experience [Feature] Fonts API labels Sep 25, 2023
@properlypurple
Copy link
Contributor

I was able to reproduce this on WP 6.4-alpha-56670, with Gutenberg 16.7.0-rc.1. The screenshot below shows a toast saying Error inserting fonts, but it's in the layer below the fonts modal, and is not very visible.

Screenshot from 2023-09-25 16-19-13

@annezazu annezazu added [Feature] Typography Font and typography-related issues and PRs and removed [Feature] Fonts API labels Sep 25, 2023
@madhusudhand
Copy link
Member Author

@jasmussen can you suggest error messaging designs and message copy for the following scenarios.

  1. All selected files uploaded successfully. (It shows Upload successful message and no designs required)
  2. All uploads might have failed.
  3. Only part of the uploads are failed (for the following reasons)

Failure reasons:

  • Unsupported file extensions (ex: .txt, .svg)
  • Invalid mime type (ex: a text file is given .ttf file extension, but mime isn't a font)
  • User missing permissions to upload
  • No write access on WP server
  • Selected list of fonts are duplicated of already installed fonts
  • Could be due to some bug/exception in the code.

@jasmussen
Copy link
Contributor

jasmussen commented Sep 26, 2023

Sure. For the time being, I would keep these as notices, right below the gray upload box. I would keep them all at the yellow warning level, unless it's an upload successful, then it can be green. As noted elsewhere, in the future we should redirect to the uploaded font in the Library tab instead of showing the green notice.

Please note that many of these messages are also to be found in the packages/media-utils/src/utils/upload-media.js file and others. In general all these messages should be the same, regardless of whether it's for fonts or images. If you know of other places where error messages live, and that place has copy that can work for you better than my suggestions here, please use those instead. Use the below suggestions only if the precise use case isn't shown. CC: @Mamaduka in case he has insights here.

Failure copy suggestions:

All selected files uploaded successfully. (It shows Upload successful message and no designs required)

"Upload successful".

All uploads might have failed.

"Upload failed".

Unsupported file extensions (ex: .txt, .svg)

"Sorry, you are not allowed to upload this file type."

Invalid mime type (ex: a text file is given .ttf file extension, but mime isn't a font)

"Sorry, you are not allowed to upload this file type."

User missing permissions to upload

"Sorry, you do not have permission to upload."

No write access on WP server

"Sorry, WordPress does not have permission to upload."

Selected list of fonts are duplicated of already installed fonts

Question, do we simply omit the duplicated fonts and upload any variants that are missing?

In that case, "Upload successful." — no reason to call this out, IMO.

Could be due to some bug/exception in the code.

"Upload failed."

@Mamaduka
Copy link
Member

I believe beside media upload util the remaining failure/validation errors are from server. Generally the _wp_handle_upload method handles media uploads in core.

@annezazu
Copy link
Contributor

Pulling this feedback from the FSE Outreach Program as these concerns are coming up there:

I tried to upload a .zip on purpose but got no errors. It may be nice to have or suggest allowed font formats and a confirmation notification when one gets uploaded.

Another piece of feedback on this note from the same call for testing:

Went to fonts.google.com and downloaded Fuggles.zip. It looks like I have to unzip the folder and then upload the Fuggles-Regular.ttf font file.

Proper error messaging would go along way, perhaps along with an "allowed file type" note.

@jasmussen
Copy link
Contributor

I want to direct attention back to the initial mockups, which currently look like this:

Overview

Note here the message below the Upload dropzone that says:

Uploaded fonts will appear up in your library and can be used in your theme after that. Formats .ttf, .woff, and .woff2 are supported.

I believe the separate upload tab is still being developed in a separate PR, so I'm sharing this mostly as a note to remember to go back to the mockups, a lot of detail is captured there.

That said, I wouldn't mind if it were possible to include the extension information in the upload notice as well. So potentially something like:

Sorry, you are not allowed to upload this file type. Formats .ttf, .woff, and .woff2 are supported.

@hanneslsm
Copy link

Failure copy suggestions:

All selected files uploaded successfully. (It shows Upload successful message and no designs required)
"Upload successful".

I find the messages too nondescriptive.
This post has excellent examples for better error messages: https://www.thegiggal.com/ux-writing-error-messages/
It'd be wonderful if we could at least include some of the points:

What happened, Reassurance, What happened and why, Help fix it, Give a way out.
image

@hanneslsm
Copy link

Also noting that in the current implementation the error messages disappear automatically after a few secs. This shouldn't happen.

@creativecoder
Copy link
Contributor

I think error messages have been generally improved since the REST API updates and refactor in Jan. Can we close this one?

@madhusudhand
Copy link
Member Author

Error messaging improved in latest releases and closing this as it is no longer applicable.

@github-project-automation github-project-automation bot moved this from Punted to 6.5 to Done in WordPress 6.4 Editor Tasks Apr 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Experience Ideas about improving block and theme developer experience [Feature] Font Library [Feature] Typography Font and typography-related issues and PRs [Type] Bug An existing feature does not function as intended
Projects
No open projects
Status: Done
Development

No branches or pull requests

8 participants