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

add gutenberg support #2017

Merged
merged 6 commits into from
Nov 10, 2018
Merged

add gutenberg support #2017

merged 6 commits into from
Nov 10, 2018

Conversation

timelsass
Copy link
Contributor

fixes #2009

@codecov
Copy link

codecov bot commented Nov 9, 2018

Codecov Report

Merging #2017 into develop will decrease coverage by 0.08%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             develop   #2017      +/-   ##
============================================
- Coverage        5.1%   5.01%   -0.09%     
- Complexity      1794    1839      +45     
============================================
  Files            143     144       +1     
  Lines           6269    6380     +111     
============================================
  Hits             320     320              
- Misses          5949    6060     +111
Impacted Files Coverage Δ Complexity Δ
core/class-kirki-modules.php 0% <ø> (ø) 11 <0> (ø) ⬇️
modules/css/class-kirki-modules-css.php 0% <0%> (ø) 49 <1> (+1) ⬆️
...odules/gutenberg/class-kirki-modules-gutenberg.php 0% <0%> (ø) 36 <36> (?)
...es/webfonts/class-kirki-modules-webfonts-local.php 0% <0%> (ø) 5% <0%> (ø) ⬇️
...es/postmessage/class-kirki-modules-postmessage.php 0% <0%> (ø) 128% <0%> (+2%) ⬆️
modules/css/class-kirki-output.php 0% <0%> (ø) 79% <0%> (+6%) ⬆️
...font-loader/class-kirki-modules-webfont-loader.php 0% <0%> (ø) 7% <0%> (ø) ⬇️
...es/webfonts/class-kirki-modules-webfonts-async.php 0% <0%> (ø) 11% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 87ee00e...d4af8f7. Read the comment docs.

@timelsass
Copy link
Contributor Author

timelsass commented Nov 9, 2018

Overview

This adds a Gutenberg module to Kirki.

#2009 mentions the style conflicts of just having the enqueue added to the enqueue_block_editor_assets hooks, which is something that will happen. Gutenberg uses a transform js method at runtime to prefix selectors, so to utilize this I added the styles to the editor's initialization settings in block_editor_settings - which is where add_editor_styles() calls are outputted. This would allow kirki styles to undergo the same transformations, which helps namespace a lot of the more general selectors like body, h2, p, etc. Ultimately, I think this is the best way to approach it as this all should lead the user back to WordPress' documentation on adding theme support for Gutenberg as the same rules apply. Most things should work, but it's up to the developers to double check for conflicts - so following the same spirit of core - the feature should be opt-in for configs passed to Kirki.

Decisions

  1. I did add this as a module, but wasn't entirely sure if this is where it was envisioned to live or if it should be added in /core.

  2. As mentioned in Gutenberg Compatibility? #2009 as this being handled by Kirki it seemed appropriate to have a config setting for the plugins and themes to opt in to this feature. I added the gutenberg_support config to be checked for the registered configs as the way to opt in.

  3. As part of Output Question #1 mentioned, this PR does have Kirki check and set the add_theme_support to opt in for the 'editor-styles' transforms.

  4. I used Kirki_Modules_Webfonts_Async to load the webfonts appropriately, but I guess other loading methods could be used. This was most convenient for me as I use that for some editor typography controls as well.

  5. Without thoroughly digging through the code, I wasn't sure when/where FontAwesome enqueue is actually being done, so I did add a getter to the CSS Module to get the $fa_enqueue var since it was protected to check and enqueue in editor_block_assets. I assumed that it's probably set properly elsewhere, but have not tested.

Testing

I was using twentynineteen as a test case with the following code added to functions.php.

Kirki::add_config( 'twentynineteen', array(
	'gutenberg_support' => true,
	'capability'  => 'edit_theme_options',
	'option_type' => 'theme_mod',
) );

Kirki::add_section( 'typography_testing', array(
	'title'    => esc_attr__( 'Typography Testing', 'twentynineteen' ),
	'priority' => 100,
) );

Kirki::add_field( 'twentynineteen', array(
	'type'        => 'typography',
	'settings'    => 'typography_testing',
	'label'       => esc_attr__( 'Control Label', 'twentynineteen' ),
	'section'     => 'typography_testing',
	'default'     => array(
		'font-family'    => 'Roboto',
		'variant'        => 'regular',
		'font-size'      => '14px',
		'line-height'    => '1.5',
		'letter-spacing' => '0',
		'color'          => '#333333',
		'text-transform' => 'none',
		'text-align'     => 'left',
	),
	'priority'    => 10,
	'output'      => array(
		array(
			'element' => 'body',
		),
	),
) );

Test opt-in support by removing the gutenberg_support key from the config / changing to false. I am only checking strict for true.

The PR should account for KIRKI_NO_OUPUT constant being defined & true, and also should check the config for disable_output being set to prevent specific configs from outputting.

This particular theme does have typography styles assigned that are more specific than just body, but you should be able to verify that the font is correctly loaded.


Note: I have done a similar implementations on some themes which worked fine for my own use cases, but I also know people extend Kirki and use it in different ways, so I could be missing somethings.

Props also to @rramo012 (github/wporg) for the idea of implementing the webfonts loader in BoldGrid/boldgrid-theme-framework@1d8c0a1

@aristath aristath merged commit 1fcb5ba into themeum:develop Nov 10, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gutenberg Compatibility?
2 participants