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

Custom Fonts API #1807

Closed
themeszone opened this issue Feb 19, 2018 · 31 comments
Closed

Custom Fonts API #1807

themeszone opened this issue Feb 19, 2018 · 31 comments

Comments

@themeszone
Copy link

Issue description:

Hello!

Is there a way to add custom fonts with variants to typography field. The old way with filter for google fonts does not seem to work anymore.

Version used:

(Did you try using the develop branch from github? There's a chance your issue has already been adressed there)

Using theme_mods or options?

using theme_mods

Code to reproduce the issue (config + field(s))

// This does not seem to work anymore

add_filter( 'kirki_fonts_google_fonts', 'ellie_custom_fonts_google');

function ellie_custom_fonts_google($fonts){
	$my_fonts['ABC'] = array(
		'label' => 'ABC',
		'variants' => array('200', '300', '400','400italic', '500','500italic', '600','600italic', '700','700italic', '800','800italic', 'regular','italic'),
		'category' => 'sans-serif',
	);

	return array_merge_recursive( $my_fonts, $fonts );
}
@vlthemes
Copy link

Same problem.

@mapsteps
Copy link
Contributor

mapsteps commented Mar 5, 2018

Yep, same here.

aristath added a commit that referenced this issue Mar 11, 2018
@aristath
Copy link
Contributor

The old filters had a lot of issues and eventually broke down completely.
I was working on a new implementation that will finally allow us to use custom fonts, optgroups, define custom font-weights and so on so I didn't have time to fix something that would eventually become obsolete.
Now you can do things on a per-control basis using the choices argument that controls have.
Example:

'choices' => array(
	'fonts' => array(
		'google'   => array( 'popularity', 60 ),
		'families' => array(
			'custom' => array(
				'text'     => 'My Custom Fonts',
				'children' => array(
					array( 'id' => 'helvetica-neue', 'text' => 'Helvetica Neue' ),
					array( 'id' => 'linotype-authentic', 'text' => 'Linotype Authentic' ),
				),
			),
		),
		'variants' => array(
			'helvetica-neue'     => array( 'regular', '900' ),
			'linotype-authentic' => array( 'regular', '100', '300' ),
		),
	),
),

This line tells the control to only show the top 60 fonts sorted by popularity:

'google'   => array( 'popularity', 60 ),

This part then adds a new optgroup to the font-families dropdown:

'custom' => array(
	'text'     => 'My Custom Fonts (demo only),
	'children' => array(
		array( 'id' => 'helvetica-neue', 'text' => 'Helvetica Neue' ),
		array( 'id' => 'linotype-authentic', 'text' => 'Linotype Authentic' ),
	),
),

screenshot at mar 11 21-04-24

Finally this part allows us to define the variants that our fonts will have:

'variants' => array(
	'helvetica-neue'     => array( 'regular', '900' ),
	'linotype-authentic' => array( 'regular', '100', '300' ),
),

The variants is not limited to the custom fonts, it can be used to change the variants of any font-family from any category.

This is a huge improvement that will allow us to use other webfont services besides google too... You can even use it to completely disable google and add a dozen custom fonts from adobe.io, fonts.com, fontsquirrel or whatever your heart desires.

@aristath
Copy link
Contributor

I forgot to say the changes for this have already been pushed to the develop branch so feel free to experiment with it.
I'm not going to announce it, put it on changelogs etc... I usually add features like that a bit sneaky for a few months to let a few people test it, I test it on my own themes too and then it's announced (if at all).
The 'google' => array( 'popularity', 60 ), for example has been there for months and has been thoroughly tested 😉

@vlthemes
Copy link

If I have many typography fields, should I add this construction (choice => ... ) to all fields?

Something like that?

function ellie_custom_fonts_google($fonts){
	$my_fonts['ABC'] = array(
		'label' => 'ABC',
		'variants' => array('200', '300', '400','400italic', '500','500italic', '600','600italic', '700','700italic', '800','800italic', 'regular','italic'),
		'category' => 'sans-serif',
	);

	return array_merge_recursive( $my_fonts, $fonts );
}

@aristath
Copy link
Contributor

The new implementation has nothing to do with the filters.

There's a bug in the filters that I didn't have time to debug.
What I posted above is a way to add custom fonts to your controls.
So you could for example write a function like this:

function ellie_custom_fonts_choices_for_control() {
	return array(
		'fonts' => array(
			'google'   => array( 'popularity', 60 ),
			'families' => array(
				'custom' => array(
					'text'     => 'Ellie Custom Fonts',
					'children' => array(
						array( 'id' => 'ABC', 'text' => 'ABC' ),
					),
				),
			),
			'variants' => array(
				'ABC' => array( '200', '300', '400','400italic', '500','500italic', '600','600italic', '700','700italic', '800','800italic', 'regular','italic' ),
			),
		),
	);
}

and then in your typography controls add

'choices' => ellie_custom_fonts_choices_for_control(),

@vlthemes
Copy link

Thank you! I'm gonna test it :)

@aristath
Copy link
Contributor

Just an additional note, the kirki_fonts_google_fonts should not be used to add custom fonts. That was never the intention behind that filter. The intended use of the kirki_fonts_google_fonts filter was to filter the google-fonts, remove some you didn't like, modify others (maybe removing a few variants you thought didn't belong in your theme) etc.
The only reason we were using that filter to add custom fonts was because we lacked a better implementation that would allow us to add new custom font categories etc.

@vlthemes
Copy link

vlthemes commented Mar 11, 2018

Is it possible to add subset?

Everything else works fine :)

P.S. When will this version be available on wp.org?

P.P.S I use dev version and this piece of code https://github.com/vlthemes/VLThemes-Add-Typekit-fonts-to-Kirki and I have this bug?
http://take.ms/7bDuB

@themeszone
Copy link
Author

Thanks a lot for the Solution!

@sergeysedykh
Copy link

The per-control solution is not very convenient, as we'll have to modify all controls one by one instead of adding a global filter.

I'm in ❤️with Kirki however and I'm up to anything.

@aristath, could you please kindly confirm, that font filters will be deprecated (at least for adding custom fonts) and the proposed per-control solution is preferable?

Thanks a lot for your time!

@sergeysedykh
Copy link

sergeysedykh commented Mar 14, 2018

@vlthemes, this is a per-control solution. You have to modify all controls by adding the choices attribute:

'choices' => ellie_custom_fonts_choices_for_control(),

As I noted, it's not very convenient, as we'll have to modify all controls one by one instead of adding a global filter.

aristath added a commit that referenced this issue Mar 17, 2018
* develop: (73 commits)
  fixes #1730
  fixes #1830
  GDPR: Load webfont-loader locally
  Update fonts
  fixes #1834
  Apply WordPress Coding Standards
  Update kirki-helper-class.md
  see #1797
  cleanup unused vars
  See #1807
  Additional fix for #1809
  fixes #1828
  fixes #1808
  fix #1814
  fix #1797
  fixes #1809
  Update sortable.md
  fixes #1787
  update webfonts & grunt
  changelog
  ...

# Conflicts:
#	modules/postmessage/class-kirki-modules-postmessage.php
@aristath
Copy link
Contributor

The per-control solution is not very convenient, as we'll have to modify all controls one by one instead of adding a global filter.

As a principle of good web design no site should use more than 2-3 font-families. one for body, another for headers (if not the same) and if they want some decoration text then perhaps a 3rd one there.
Having that in mind there will not be a global option as typography controls should be kept to a minimum. But even if there are 10 typography controls, adding a line to each one of them shouldn't be too much trouble.

@aristath, could you please kindly confirm, that font filters will be deprecated (at least for adding custom fonts) and the proposed per-control solution is preferable?

The filters will not be removed. They still have a purpose to serve, they will simply not be abused like before 😆. For adding font-families etc however, the per-control solution is definitely preferable.

@sergeysedykh
Copy link

As a principle of good web design no site should use more than 2-3 font-families.

Completely agree.

Thanks! Will stick to the new per-control solution.

@sergeysedykh
Copy link

Added user fonts via a custom function and the choices parameter. Thanks!

What's the purpose of the Default CSS Values?

There's an empty selection in the section too:

image

Can this section be removed?

@aristath
Copy link
Contributor

aristath commented Mar 21, 2018

The empty option simply doesn't add a font-family (an empty font-family is not the same as inherit). I just need to add an actual title for that... https://github.com/aristath/kirki/blob/86c5a1231625efbbf006cbd9fdc9012ee0864619/controls/js/src/typography.js#L138-L141

@sergeysedykh
Copy link

The "Default CSS Values" option would look scary for novice users. Don't have a better idea for the title though :(

I would probably exclude the section altogether or merge it with the Default Fonts.

Probably, consider adding a filter for removing the section?

@mapsteps
Copy link
Contributor

Hey @aristath,

I'm having a little bit of trouble here.

With the new implementation, I can't define a "stack" for my custom fonts.

This is what I was using so far to replace the default standard fonts:

function prefix_custom_default_fonts( $standard_fonts ) {

	$fonts = array();

	$fonts['helvetica_neue'] = array(
		'label' => 'Helvetica Neue',
		'variants' => array( 'regular', 'italic', '700', '700italic' ),
		'stack' => '"Helvetica Neue", Helvetica, Arial, sans-serif',
	);

	$fonts['helvetica'] = array(
		'label' => 'Helvetica',
		'variants' => array( 'regular', 'italic', '700', '700italic' ),
		'stack' => 'Helvetica, Arial, sans-serif',
	);

	$fonts['arial'] = array(
		'label' => 'Arial',
		'variants' => array( 'regular', 'italic', '700', '700italic' ),
		'stack' => 'Arial, Helvetica, sans-serif',
	);

	return $fonts;

}

add_filter( 'kirki/fonts/standard_fonts', 'prefix_custom_default_fonts', 0 );

Now I can either use the implementation shown above (which adds another "Standard Fonts" group) or use this from the docs:

// If you want to use custom definitions you can also do that:
'choices' => array(
	'fonts' => array(
		'standard' => array(
			'Georgia,Times,"Times New Roman",serif',
			'Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif'
		),
	),
),

which doesn't give me the option to define a name for the font. The output looks something like this:

bildschirmfoto 2018-04-11 um 11 05 01

Is it fine to stick to the old implementation in that case or is there a recommended way to do this?

@mapsteps
Copy link
Contributor

+1 for a filter to remove the "Default CSS Values" group. (maybe there already is one, I just haven't had the chance yet to dig deeper) lol

@vlthemes
Copy link

Not working again.

@mapsteps
Copy link
Contributor

Hey @vlthemes,

are you using the latest dev or stable version?

I'm having trouble with the stable version, everything seems to work great with the one in production though.

@vlthemes
Copy link

Hi!

It works on dev only.

But my clients use stable version from wp repo. And it not works for their.

@mapsteps
Copy link
Contributor

Yeah, I'm shipping it with the theme but also use the latest stable version.

Patiently waiting for the next release :D

@vlthemes
Copy link

@aristath please, release new version with this commit :)

@aristath
Copy link
Contributor

I think this is the only outstanding bug that currently has to be resolved... #1837 As soon as that's done I'll be releasing v3.0.26 👍

@sergeysedykh
Copy link

@mapsteps did you find a way to add "stacks" to custom fonts? @aristath, any hints? Thanks!

@mapsteps
Copy link
Contributor

mapsteps commented May 16, 2018

hi @classyentrepreneur,

unfortunately not. I'm still having a few problems with the new implementation.

Scenario:

What would I do if I'd like to add a custom optgroup for Typekit fonts and let's another one for custom fonts and replace/extend the default fonts with my own set of defaults as described here.

I tried to use the following code as a starting point and work with filters but couldn't get it to work properly.

function ellie_custom_fonts_choices_for_control() {
	return array(
		'fonts' => array(
			'google'   => array( 'popularity', 60 ),
			'families' => array(
				'custom' => array(
					'text'     => 'Ellie Custom Fonts',
					'children' => array(
						array( 'id' => 'ABC', 'text' => 'ABC' ),
					),
				),
			),
			'variants' => array(
				'ABC' => array( '200', '300', '400','400italic', '500','500italic', '600','600italic', '700','700italic', '800','800italic', 'regular','italic' ),
			),
		),
	);
}

I think it would be easier to filter if the variants array was inside the families array. Wouldn't that make more sense to add additional optgroups?

I feel like I'm missing or overlooking something. Any idea @aristath? Would love to see an example on how you'd tackle this.

@mapsteps
Copy link
Contributor

Created a separate issue for this since my problem a bit more complex – #1900

@pkohara
Copy link

pkohara commented Sep 5, 2019

Hi, I am wondering if someone can help, I have been sent this link as I would like to change the typography of my site title website (Di Blog through Wordpress) to a custome font (Hesterica or Shadeerah) but it is currently only offering me google fonts.

I've tried to use the code above into the addition CSS but it says there are 94 errors and therefore won't save it. My understanding of code is very limited. What am I doing wrong?

Thank-you,
Pamela

@aristath
Copy link
Contributor

aristath commented Sep 7, 2019

@pkohara I'm afraid without details about the specif errors you're getting there's not much we can do to understand what's wrong and help.

Please create a new issue so we can examine it there.

@mapsteps
Copy link
Contributor

Should be fixed with the upcoming v4 release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants