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 Picker: Use searchable dropdown #3027

Closed
swissspidy opened this issue Aug 13, 2019 · 15 comments · Fixed by #3155 or #3218
Closed

Font Picker: Use searchable dropdown #3027

swissspidy opened this issue Aug 13, 2019 · 15 comments · Fixed by #3155 or #3218
Labels
Enhancement New feature or improvement of an existing one
Milestone

Comments

@swissspidy
Copy link
Collaborator

See #2877 for details.

When we have all these new fonts, we need a better font picker that easily allows scrolling through the hundreds of fonts.

accessible-autocomplete seems like a good candidate for me, but perhaps there is also a Gutenberg-originating component that we can use.

@spacedmonkey
Copy link
Contributor

Why do we need to to import another library here?

There are a couple of autocomplete in gutenberg already.

Screenshot from 2019-08-22 15-08-15
The url autocomplete on button.

Screenshot from 2019-08-22 15-05-44
Tag / category / tax picker autocomplete.

I wonder if we can use either of them.

@swissspidy
Copy link
Collaborator Author

swissspidy commented Aug 22, 2019

We need a list of all ~1000 fonts that you can scroll through but also filter by typing. These two components don't really allow for that.

The suggestion to use accessible-autocomplete came after following development on WordPress/gutenberg#7384 and WordPress/gutenberg#7385, where this library was evaluated as being the best candidate for this scenario and also accessible.

@spacedmonkey
Copy link
Contributor

Isn't there an issue here. If the font picker is a search box, doesn't that require the user to know the name of the font they need? It means users can just browser the fonts they need from a dropdown like google docs or Word.

I had a quick look at google docs. See attached screenshots of how it's font selector works.

Screenshot from 2019-08-30 09-31-02

More font options

Screenshot from 2019-08-30 09-31-14

This may not be required, but I thought flag it

@swissspidy
Copy link
Collaborator Author

If the font picker is a search box, doesn't that require the user to know the name of the font they need?

As noted in my previous comment:

We need a list of all ~1000 fonts that you can scroll through but also filter by typing.

So it would not be search-only.


Thanks for sharing these screenshots from Google Docs! I took it as an opportunity to see how they load the font previews.

Turns out, they just (lazy) load all the fonts, but only with minimum amount of glyphs required. For example, to preview the Slabo 27px font, they basically load https://fonts.googleapis.com/css?family=Slabo+27px&display=swap&text=Slabo%2027px, which points to a ~1KB WOFF2 file that only includes the characters ablopSx27 .

@spacedmonkey
Copy link
Contributor

For the autocomplete for the font autocomplete. Do we think that a no font found message is required? Or should nothing is displayed if no results are found?

I have mocked it up.
Screenshot from 2019-09-03 14-45-00

Not sure if it required or not.

@swissspidy
Copy link
Collaborator Author

That is reasonable, yes. Looks like the autocomplete component supports a tNoResults prop.

@spacedmonkey
Copy link
Contributor

That is reasonable, yes. Looks like the autocomplete component supports a tNoResults prop.

Add here - 5c92430

@swissspidy swissspidy added this to the v1.3 milestone Sep 9, 2019
@swissspidy
Copy link
Collaborator Author

@spacedmonkey Could you please add some testing instructions here for Claudio and then move this to the QA column on the board? Thanks!

@swissspidy
Copy link
Collaborator Author

swissspidy commented Sep 10, 2019

I just ran into something odd:

  1. Create new story
  2. Write some text in the text block
  3. Change font to some Google font
  4. Save draft
  5. See block validation error due to excessive CSS 🤔
  6. Clear font using the X icon button
  7. Notice that the "Save Draft" option does not appear, despite data being changed. 🤔
  8. Click on "Preview Post"
  9. Save Draft
  10. Editor crashes 🤔
  11. Also notice a few prop types warnings in the browser console 🤔

While I was able to fix some of the errors with the following diff, it still crashes due to an issue in the original Autocomplete itself.

diff --git assets/src/stories-editor/components/font-family-picker/autocomplete.js assets/src/stories-editor/components/font-family-picker/autocomplete.js
index b8398ee8..253d8529 100644
--- assets/src/stories-editor/components/font-family-picker/autocomplete.js
+++ assets/src/stories-editor/components/font-family-picker/autocomplete.js
@@ -51,8 +51,8 @@ class Autocomplete extends OriginalAutocomplete {
 
 		const inputFocused = focused === -1;
 		const noOptionsAvailable = options.length === 0;
-		const queryNotEmpty = query.length !== 0;
-		const queryLongEnough = query.length >= minLength;
+		const queryNotEmpty = query && query.length !== 0;
+		const queryLongEnough = query && query.length >= minLength;
 		const showNoOptionsFound = this.props.showNoOptionsFound &&
 			inputFocused && noOptionsAvailable && queryNotEmpty && queryLongEnough;
 
@@ -84,7 +84,7 @@ class Autocomplete extends OriginalAutocomplete {
 			<div className={ wrapperClassName } onKeyDown={ this.handleKeyDown } role="combobox" tabIndex="-1" aria-expanded={ menuOpen ? 'true' : 'false' }>
 				<Status
 					length={ options.length }
-					queryLength={ query.length }
+					queryLength={ query && query.length }
 					minQueryLength={ minLength }
 					selectedOption={ this.templateInputValue( options[ selected ] ) }
 					selectedOptionIndex={ selected }

The excessive CSS thing is also very odd thing by itself. It even happens when choosing "Arial" as the custom font. No idea what's going on there.

Screenshots (click to expand)

Screenshot 2019-09-10 at 10 46 45
Screenshot 2019-09-10 at 10 46 39
Screenshot 2019-09-10 at 10 43 32
Screenshot 2019-09-10 at 10 49 56
Screenshot 2019-09-10 at 10 49 46

@spacedmonkey
Copy link
Contributor

I have created a PR with some small tweaks that would fix that above error message ^
#3218

@spacedmonkey
Copy link
Contributor

Steps to test.

  1. Create new story
  2. Write some text in the text block
  3. Type in font picker. Select a font.
  4. Save draft
  5. Check font is loaded.
  6. Clear font using X.
    1, Select another font.
  7. Publish story.

@csossi
Copy link

csossi commented Sep 11, 2019

Verified in QA

@spacedmonkey
Copy link
Contributor

Screencast can be seen here. https://youtu.be/D8Yg7Yv4Los ( Same screencast as #3026 )

@pbakaus
Copy link

pbakaus commented Sep 17, 2019

hey folks, is there a reason there's no actual dropdown button, for better (mouse) discoverability? Was this a conscious decision?

@spacedmonkey
Copy link
Contributor

hey folks, is there a reason there's no actual dropdown button, for better (mouse) discoverability? Was this a conscious decision?

This was a conscious decision. Adding in a dropdown made for a bad user experience. For a number of reasons.

  • A scrollable div with 960+ font in it, isn't specially usable.
  • Dropdown icon, made it easily unclear that a user can type inside, what looks like a dropdown / select field.
  • A large dropdown, makes it hard to select the 'None' option.
  • A dropdown with 960+ fonts it, would require loading either an the font files for all options, meaning 960+ requests in the browser. Either that, or a complex system of lazy loading in font options. This would require create / designing and testing a new custom component. This would have added multiple days to the build time.

I recommended added an option that show 'Popular Fonts' when user focuses on the input. But UX would need to be designed for this.

I didn't do the original do the discovery on this one. I just implied what was on the ticket.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or improvement of an existing one
Projects
None yet
5 participants