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

Use autocomplete for font selection. #3155

Merged
merged 54 commits into from
Sep 10, 2019
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
673d31c
Use autocomplete for font selection.
spacedmonkey Aug 30, 2019
aab610e
Not need to remove this logic. It doesn't hurt to check if font are l…
spacedmonkey Sep 2, 2019
6fbec70
Fix object being passed.
spacedmonkey Sep 2, 2019
f958f54
Tidy up dependencies.
spacedmonkey Sep 2, 2019
304892d
CSS tidy up.
spacedmonkey Sep 2, 2019
f633c62
Merge branch 'develop' into feature/autocomplete
swissspidy Sep 2, 2019
7e086b3
Add font picker e2e tests and remove unneeds junit tests.
spacedmonkey Sep 3, 2019
ee3545a
Merge remote-tracking branch 'origin/feature/autocomplete' into featu…
spacedmonkey Sep 3, 2019
b085055
Make the compound more generic. Add help text.
spacedmonkey Sep 3, 2019
5c92430
Change to add no font found options.
spacedmonkey Sep 3, 2019
2b91bb5
Fix typo.
spacedmonkey Sep 3, 2019
a27364f
Test that google fonts are loaded.
spacedmonkey Sep 3, 2019
344bfa5
Fix spaces.
spacedmonkey Sep 3, 2019
4e8fac2
Introduce amp_story_head action
westonruter Aug 17, 2019
64b3b8d
Introduce amp_get_boilerplate_stylesheets() function for returning in…
westonruter Aug 22, 2019
fd253c9
Discontinue injecting style[amp-custom] and boilerplate at wp_head ac…
westonruter Aug 22, 2019
b836c9e
Add AMP v0.js during post-processing instead of wp_head action
westonruter Aug 22, 2019
910bd32
Ensure HTML fragments get proper encoding for DOM parsing
westonruter Aug 22, 2019
ed2ac0e
Add todo comment about removing unused scripts
westonruter Aug 22, 2019
e2ad188
Prevent post-processing responses as AMP pages if not well-formed HTM…
westonruter Aug 27, 2019
49b69d4
Improve order and comments for amp_story_head actions
westonruter Aug 28, 2019
88f0da9
Update block-editor package
swissspidy Aug 30, 2019
9537748
Import core-data module to ensure data store is always available
swissspidy Aug 30, 2019
e863b68
Make <head> regex more robust
westonruter Aug 30, 2019
3bc3adf
Unset amp-runtime script after it has been added to ordered scripts
westonruter Aug 30, 2019
3b63efb
Fix comment typo in AMP_Theme_Support::prepare_response()
westonruter Aug 30, 2019
87f0b8d
Bump requried Gutenberg version by one
swissspidy Sep 2, 2019
e807df5
Use Jest silent reporter to reduce noise in output (#3165)
swissspidy Sep 2, 2019
ca33976
Update dependency @wordpress/scripts to v4.1.0 (#3169)
renovate-bot Sep 3, 2019
591154d
Fix flaky templates test (#3166)
miina Sep 3, 2019
f6d2865
Hide reusable blocks from story editor global inserter. (#3170)
miina Sep 3, 2019
8c577ff
Exclude file_exists() checks from plugin builds
westonruter Sep 3, 2019
d3434de
Remove launched experiment amp-img-auto-sizes
westonruter Sep 3, 2019
7fdd53c
Add missing amp_story_head actions present on wp_head (#3175)
westonruter Sep 4, 2019
45058c0
Font Picker: Bundle list of Google Fonts (#3110)
spacedmonkey Sep 4, 2019
f10a177
Fix translator comments
swissspidy Sep 4, 2019
a5cb2c7
Use withInstanceId HOC
swissspidy Sep 4, 2019
81e6333
Fix issues after rebase
swissspidy Sep 4, 2019
ced8932
Merge branch 'develop' into feature/autocomplete
swissspidy Sep 4, 2019
a9ddd23
Change autocomplete to inline to fix scrolling issue.
spacedmonkey Sep 5, 2019
53f7c47
Extend Autocomplete
spacedmonkey Sep 5, 2019
6c48a9f
Tidy up import / export of override.
spacedmonkey Sep 5, 2019
7040815
Fix e2e tests.
spacedmonkey Sep 5, 2019
17dceec
Use a variable.
spacedmonkey Sep 5, 2019
2d6a6d7
Rename overridden component to align with rest of project
swissspidy Sep 5, 2019
10530cb
Change displayMenu to overlay
swissspidy Sep 5, 2019
3933a91
Change placeholder
swissspidy Sep 5, 2019
74534d0
Rename some vars to make it more clear what we’re dealing with
swissspidy Sep 5, 2019
302056c
Add clear font button
spacedmonkey Sep 9, 2019
6f9e7b6
Remove px.
spacedmonkey Sep 9, 2019
f87c00a
Use to be null.
spacedmonkey Sep 9, 2019
37a3355
Updated Status component to simple functional comp
barklund Sep 9, 2019
2bb80cf
Merge branch 'develop' into feature/autocomplete
swissspidy Sep 10, 2019
f0f28b8
Fix indentation
swissspidy Sep 10, 2019
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
33 changes: 33 additions & 0 deletions assets/src/stories-editor/components/font-family-picker/edit.css
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
.autocomplete__wrapper .autocomplete__menu {
border-color: #007cba;
width: 101%;
border-bottom-left-radius: 4px !important;
border-bottom-right-radius: 4px !important;
margin-top: -3px;
padding-top: 3px;
}

.autocomplete__icon {
position: absolute;
top: 4px;
right: 4px;
}

.autocomplete__option {
border-bottom: 0 none;
padding: 6px 8px;
font-size: 13px;
}

.autocomplete__option--focused,
.autocomplete__option:hover {
background-color: #0071a1;
border-color: #0071a1;
}

.autocomplete__wrapper .autocomplete__input.autocomplete__input--focused {
color: #191e23;
border-color: #007cba;
box-shadow: 0 0 0 1px #007cba;
outline: 2px solid transparent;
}
97 changes: 62 additions & 35 deletions assets/src/stories-editor/components/font-family-picker/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,17 +2,20 @@
* External dependencies
*/
import PropTypes from 'prop-types';
import Autocomplete from 'accessible-autocomplete/react';

/**
* WordPress dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { __, _n, sprintf } from '@wordpress/i18n';
import { BaseControl } from '@wordpress/components';

/**
* Internal dependencies
spacedmonkey marked this conversation as resolved.
Show resolved Hide resolved
*/
import { AMP_STORY_FONT_IMAGES } from '../../constants';
import { PreviewPicker } from '../';
import { maybeEnqueueFontStyle } from '../../helpers';
import 'accessible-autocomplete/src/autocomplete.css';
import './edit.css';

/**
* Font Family Picker component.
Expand All @@ -23,50 +26,74 @@ function FontFamilyPicker( {
fonts = [],
onChange = () => {},
value = '',
id = '',
} ) {
const defaultOption = {
value: '',
label: __( 'None', 'amp' ),
const results = fonts;
const suggest = ( query, syncResults ) => {
const searchResults = query ? results.filter( function( result ) {
return result.name.toLowerCase().indexOf( query.toLowerCase() ) !== -1;
} ) :
[];
syncResults( searchResults );
};

const options = fonts.map( ( font ) => ( {
value: font.name,
label: font.name,
} ) );
const suggestionTemplate = ( result ) => {
maybeEnqueueFontStyle( result.name );
Copy link
Collaborator

Choose a reason for hiding this comment

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

This loads the whole font stylesheet, whereas for performance reasons it might be beneficial to only load the glyphs actually needed to display the font name.

See #3027 (comment) for an example.

Then, when the font has been selected, the whole font stylesheet could be loaded.

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 did some profiling here, the font is only 24kb, which is the size of a small image. As this is only loaded once the font is being displayed, it also has the benefit the preload the font before when displayed in the content of the amp page. Even 10 fonts loaded is only 240kb, which is the size of a small image.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm that sounds promising. I'll check it out myself while reviewing.

const fallbacks = ( result.fallbacks ) ? ', ' + result.fallbacks.join( ', ' ) : '';
return result && `<span style='font-family: ${ result.name }${ fallbacks }'>${ result.name }</span>`;
};

const fontLabel = ( familyName ) => AMP_STORY_FONT_IMAGES[ familyName ] ?
AMP_STORY_FONT_IMAGES[ familyName ]( { height: 13 } ) :
familyName;
const inputValueTemplate = ( result ) => {
return result && result.name;
};

return (
<PreviewPicker
value={ value }
options={ options }
defaultOption={ defaultOption }
onChange={ ( { value: selectedValue } ) => onChange( '' === selectedValue ? undefined : selectedValue ) }
<BaseControl
label={ __( 'Font Family', 'amp' ) }
id="amp-stories-font-family-picker"
ariaLabel={ ( currentOption ) => {
return sprintf(
/* translators: %s: font name */
__( 'Font Family: %s', 'amp' ),
currentOption.label
);
} }
renderToggle={ ( { label } ) => fontLabel( label ) }
renderOption={ ( option ) => {
return (
<span className="components-preview-picker__dropdown-label" data-font-family={ option.value === '' ? undefined : option.value }>
{ fontLabel( option.label ) }
</span>
);
} }
/>
id={ id }
help={ __( 'Type to search for fonts', 'amp' ) }
>
<Autocomplete
id={ id }
source={ suggest }
templates={
{
suggestion: suggestionTemplate,
inputValue: inputValueTemplate,
}
}
minLength={ 2 }
onConfirm={ onChange }
showAllValues={ false }
confirmOnBlur={ false }
defaultValue={ value }
dropdownArrow={ () => '' }
preserveNullOptions={ true }
placeholder={ __( 'None', 'amp' ) }
displayMenu="overlay"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why was this changed? It fixes the scrolling issue I spent all of today working on. Please stop making changes like this.

showNoOptionsFound={ false }
tStatusQueryTooShort={ ( minQueryLength ) =>
// translators: %d: the number characters required to initiate an author search.
barklund marked this conversation as resolved.
Show resolved Hide resolved
sprintf( __( 'Type in %d or more characters for results', 'amp' ), minQueryLength )
}
// translators: 1: the index of thre selected result. 2: The total number of results.
spacedmonkey marked this conversation as resolved.
Show resolved Hide resolved
tStatusSelectedOption={ ( selectedOption, length ) =>
sprintf( __( '%1$s (1 of %2$s) is selected', 'amp' ), selectedOption, length )
}
tStatusResults={ ( length, contentSelectedOption ) => {
return (
_n( '%d font is available.', '%d fonts are available.', length, 'amp' ) +
' ' + contentSelectedOption
);
} }
/>
</BaseControl>
);
}

FontFamilyPicker.propTypes = {
value: PropTypes.string,
id: PropTypes.string,
fonts: PropTypes.arrayOf( PropTypes.shape( {
value: PropTypes.string,
label: PropTypes.string,
Expand Down

This file was deleted.

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -432,10 +432,11 @@ export default createHigherOrderComponent(
<FontFamilyPicker
fonts={ ampStoriesFonts }
value={ ampFontFamily }
onChange={ ( value ) => {
maybeEnqueueFontStyle( value );
setAttributes( { ampFontFamily: value } );
onChange={ ( font ) => {
maybeEnqueueFontStyle( font.name );
setAttributes( { ampFontFamily: font.name } );
} }
id={ 'amp-stories-font-family-picker' }
/>
<ToggleControl
label={ __( 'Automatically fit text to container', 'amp' ) }
Expand Down
15 changes: 15 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
"@wordpress/server-side-render": "1.1.0",
"@wordpress/url": "2.7.0",
"@wordpress/wordcount": "2.6.0",
"accessible-autocomplete": "1.6.2",
"autoprefixer": "9.6.1",
"babel-eslint": "10.0.3",
"babel-jest": "24.9.0",
Expand Down
Loading