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

Fix Typography panel rendering from style hooks #22605

Merged
merged 7 commits into from
Jun 11, 2020

Conversation

ItsJonQ
Copy link

@ItsJonQ ItsJonQ commented May 25, 2020

This update fixes the Typography sidebar panel rendering, specifically if line-height/font-size style properties are disabled.

The Typography panel will no longer render if it does not contain any style controls.

How has this been tested?

This has been tested locally within Gutenberg.

  • Run npm run dev

  • Add add_theme_support( 'disable-custom-line-height' ) to your theme's function.php

  • Add a Paragraph block

  • Typography panel should still render, but without line height controls

  • Remove font-sizes for Paragraph and/or Heading block

  • Refresh

  • Ensure that there is no Typography panel

Screenshots

Screen Shot 2020-05-25 at 12 35 47 PM

Types of changes

  • Create a Typography panel component under hooks/ that handles the rendering of typography style controls

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

Resolves: #22602

This update fixes the Typography sidebar panel rendering, specifically if line-height/font-size style properties are disabled.
The Typography panel will no longer render if it does not contain any style controls.
@ItsJonQ ItsJonQ added [Type] Bug An existing feature does not function as intended [Block] Heading Affects the Headings Block [Block] Paragraph Affects the Paragraph Block labels May 25, 2020
@ItsJonQ ItsJonQ requested review from youknowriad and gziolo May 25, 2020 16:52
@ItsJonQ ItsJonQ requested a review from ellatrix as a code owner May 25, 2020 16:52
@ItsJonQ ItsJonQ self-assigned this May 25, 2020
@github-actions
Copy link

github-actions bot commented May 25, 2020

Size Change: +10.1 kB (0%)

Total Size: 1.13 MB

Filename Size Change
build/annotations/index.js 3.62 kB -1 B
build/api-fetch/index.js 3.4 kB +1 B
build/autop/index.js 2.83 kB +2 B (0%)
build/block-directory/index.js 7.22 kB +470 B (6%) 🔍
build/block-editor/index.js 106 kB +525 B (0%)
build/block-editor/style-rtl.css 12.1 kB +676 B (5%) 🔍
build/block-editor/style.css 12.1 kB +670 B (5%) 🔍
build/block-library/editor-rtl.css 7.88 kB +10 B (0%)
build/block-library/editor.css 7.89 kB +10 B (0%)
build/block-library/index.js 129 kB +2.35 kB (1%)
build/block-library/style-rtl.css 7.96 kB +271 B (3%)
build/block-library/style.css 7.96 kB +275 B (3%)
build/block-serialization-default-parser/index.js 1.88 kB -1 B
build/block-serialization-spec-parser/index.js 3.1 kB +1 B
build/blocks/index.js 48.1 kB -41 B (0%)
build/components/index.js 195 kB +1.92 kB (0%)
build/components/style-rtl.css 19.5 kB +18 B (0%)
build/components/style.css 19.5 kB +13 B (0%)
build/compose/index.js 9.31 kB +3 B (0%)
build/core-data/index.js 11.4 kB -9 B (0%)
build/data-controls/index.js 1.29 kB -2 B (0%)
build/data/index.js 8.44 kB -12 B (0%)
build/date/index.js 5.47 kB +1 B
build/deprecated/index.js 772 B +1 B
build/dom/index.js 3.17 kB -1 B
build/edit-navigation/index.js 8.26 kB +10 B (0%)
build/edit-navigation/style-rtl.css 975 B +57 B (5%) 🔍
build/edit-navigation/style.css 974 B +55 B (5%) 🔍
build/edit-post/index.js 303 kB +298 B (0%)
build/edit-post/style-rtl.css 5.6 kB +168 B (3%)
build/edit-post/style.css 5.6 kB +168 B (3%)
build/edit-site/index.js 16.6 kB +1.64 kB (9%) 🔍
build/edit-widgets/index.js 9.34 kB +507 B (5%) 🔍
build/editor/editor-styles-rtl.css 423 B -2 B (0%)
build/editor/editor-styles.css 423 B -5 B (1%)
build/editor/index.js 44.8 kB +122 B (0%)
build/element/index.js 4.64 kB -2 B (0%)
build/format-library/index.js 7.72 kB +2 B (0%)
build/hooks/index.js 2.13 kB -4 B (0%)
build/html-entities/index.js 622 B +1 B
build/i18n/index.js 3.56 kB +1 B
build/is-shallow-equal/index.js 710 B -1 B
build/keyboard-shortcuts/index.js 2.51 kB -4 B (0%)
build/keycodes/index.js 1.94 kB -2 B (0%)
build/list-reusable-blocks/index.js 3.12 kB -1 B
build/media-utils/index.js 5.29 kB -4 B (0%)
build/notices/index.js 1.79 kB -1 B
build/nux/index.js 3.4 kB -4 B (0%)
build/plugins/index.js 2.56 kB -2 B (0%)
build/server-side-render/index.js 2.68 kB +2 B (0%)
build/token-list/index.js 1.28 kB -1 B
build/viewport/index.js 1.85 kB -1 B
build/warning/index.js 1.14 kB +1 B
build/wordcount/index.js 1.17 kB -1 B
ℹ️ View Unchanged
Filename Size Change
build/a11y/index.js 1.14 kB 0 B
build/blob/index.js 620 B 0 B
build/block-directory/style-rtl.css 892 B 0 B
build/block-directory/style.css 892 B 0 B
build/block-library/theme-rtl.css 684 B 0 B
build/block-library/theme.css 686 B 0 B
build/dom-ready/index.js 568 B 0 B
build/edit-site/style-rtl.css 2.96 kB 0 B
build/edit-site/style.css 2.96 kB 0 B
build/edit-widgets/style-rtl.css 2.4 kB 0 B
build/edit-widgets/style.css 2.4 kB 0 B
build/editor/style-rtl.css 4.26 kB 0 B
build/editor/style.css 4.27 kB 0 B
build/escape-html/index.js 733 B 0 B
build/format-library/style-rtl.css 502 B 0 B
build/format-library/style.css 502 B 0 B
build/list-reusable-blocks/style-rtl.css 226 B 0 B
build/list-reusable-blocks/style.css 226 B 0 B
build/nux/style-rtl.css 616 B 0 B
build/nux/style.css 613 B 0 B
build/primitives/index.js 1.5 kB 0 B
build/priority-queue/index.js 789 B 0 B
build/redux-routine/index.js 2.85 kB 0 B
build/rich-text/index.js 14.8 kB 0 B
build/shortcode/index.js 1.7 kB 0 B
build/url/index.js 4.06 kB 0 B

compressed-size-action

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

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

Hi @ItsJonQ,
I used the following code to disable font size and line-height picking:

function disable_typography() {
	add_theme_support( 'disable-custom-line-height' );
	add_theme_support( 'disable-custom-font-sizes' );
	add_theme_support( 'editor-font-sizes', array() );
}
add_action( 'after_setup_theme', 'disable_typography' );

But the Typography panel still appeared (empty). I guess the panel should not appear at all. Do we have a bug, or I'm missing something in my tests?

<LineHeightEdit { ...props } />
</>
);
const hasContent = !! contentMarkup;
Copy link
Contributor

Choose a reason for hiding this comment

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

Would this be always true?

Copy link
Author

Choose a reason for hiding this comment

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

🤦 Indeed. I think I had a false positive in my testing due to local Docker issues

Thanks for the heads up!

@ItsJonQ
Copy link
Author

ItsJonQ commented May 26, 2020

@youknowriad + @jorgefilipecosta Thanks for the feedback! I'm currently attempting to test + fix. My local 🐳 Docker environment is having some problems 🙃

Will update as soon as I can!

@ItsJonQ
Copy link
Author

ItsJonQ commented May 26, 2020

(I was able to get Docker behaving again)

@jorgefilipecosta I had a quick question re: add_theme_support( 'editor-font-sizes', array() );

It doesn't seem to work for me 🤔

I'm getting an array of the 5 default sizes when I do this:

	const { fontSizes } = useSelect( ( select ) =>
		select( 'core/block-editor' ).getSettings()
	);

As an aside... Originally, I wanted the TypographyPanel to be less involved in tracking either its internal elements need to appear or not (and render accordingly).

However, based on deeper exploration, I'm not sure if that can happen.

To hide the panel, I think the TypographyPanel needs to check...

  • if settings.fontSizes is empty
  • if settings.disableCustomFontSizes is true
  • if settings. __experimentalDisableCustomLineHeight is true
  • if LINE_HEIGHT_SUPPORT_KEY and FONT_SIZE_SUPPORT_KEY are disabled.

@jorgefilipecosta
Copy link
Member

jorgefilipecosta commented May 27, 2020

@jorgefilipecosta I had a quick question re: add_theme_support( 'editor-font-sizes', array() );

It doesn't seem to work for me 🤔

Could you search for editor-font-sizes in your theme folder? Maybe your theme is defining a set of font sizes in some place of the code?

I think it would also be good to run the support after theme setup something like:

function disable_typography() {
	add_theme_support( 'editor-font-sizes', array() );
}
add_action( 'after_setup_theme', 'disable_typography' );

@youknowriad
Copy link
Contributor

youknowriad commented May 27, 2020

In the "mu-plugins" loaded in the default wp-env, there's one plugin that resets the colors and font sizes I think.

@ItsJonQ
Copy link
Author

ItsJonQ commented May 28, 2020

@jorgefilipecosta + @youknowriad Hmm 🤔

I tried using an older theme (Twenty Eleven). I've added the disable hook at the end of the functions.php. The font sizes are still showing up (both in the useSelect data and in the UI)

Screen Shot 2020-05-28 at 3 41 27 PM

In the "mu-plugins" loaded in the default wp-env

Screen Shot 2020-05-28 at 3 42 14 PM

These are the only plugins I can see that are active 😊 . How may I locate this mu-plugin and/or adjust the color and font settings?

(Apologies in advance for the hassle 🙏 )

@ItsJonQ ItsJonQ added the [Status] In Progress Tracking issues with work in progress label May 28, 2020
@youknowriad
Copy link
Contributor

youknowriad commented May 29, 2020

@ItsJonQ There's a "Must-use" link at the top of your screenshot, you'll see them there. Or alternatively, there's a file called normalize-theme.php on the repo, comment the function calls there.

@ItsJonQ
Copy link
Author

ItsJonQ commented Jun 1, 2020

@youknowriad Ah! Thank you 🙏

@ItsJonQ
Copy link
Author

ItsJonQ commented Jun 1, 2020

@youknowriad + @jorgefilipecosta Haiii :D

I just pushed up a change which I think should work. However, I'm still having trouble with the add_theme_support( 'editor-font-sizes', array() ); config.

I can't seem to make the fontSizes setting empty.

I located the normalize-theme.php file (wow! super handy), and was able to add my settings there.

I was able to disable line height + custom font sizes. However, I'm not able to remove (or even add) font sizes 🤔

For example, this doesn't seem to work for me:

add_theme_support( 'editor-font-sizes', array(
	array(
		'name' => 'Small',
		'size' => 12,
		'slug' => 'small'
	),
) );

Any insight would be helpful 🙏 !
Thank you!

@ItsJonQ
Copy link
Author

ItsJonQ commented Jun 10, 2020

@youknowriad Bumping for visibility 😊

@youknowriad
Copy link
Contributor

Maybe I'm doing something wrong but I can't get the line height to disappear

function disable_typography() {
	add_theme_support( 'disable-custom-line-height' );
	add_theme_support( 'disable-custom-font-sizes' );
	add_theme_support( 'editor-font-sizes', array() );
}
add_action( 'init', 'disable_typography' );

@ItsJonQ
Copy link
Author

ItsJonQ commented Jun 10, 2020

@youknowriad Ah! Thank you for following up.

It looks like the line-height disable checking logic was using:

useEditorFeature();

rather than:

useSelect( ( select ) => select( 'core/block-editor' ).getSettings()

I made the update and pushed it up!

@youknowriad youknowriad reopened this Jun 11, 2020
@youknowriad
Copy link
Contributor

Sorry wrong click.

So, now the link control is gone but the typography panel remains empty.

@ItsJonQ
Copy link
Author

ItsJonQ commented Jun 11, 2020

@youknowriad Thank you for re-checking! I was able to test and get things working. I pushed up a fix.

Previously, I think I was getting false results because of my Docker environment. I reset some things this week, and it seemed be working okay.

Copy link
Contributor

@youknowriad youknowriad left a comment

Choose a reason for hiding this comment

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

This works great thanks for the fixes.

@ItsJonQ
Copy link
Author

ItsJonQ commented Jun 11, 2020

@youknowriad Thank you 🙏

@ItsJonQ ItsJonQ merged commit 2e639e4 into master Jun 11, 2020
@ItsJonQ ItsJonQ deleted the fix/typography-panel-rendering-when-disabled branch June 11, 2020 16:43
@github-actions github-actions bot added this to the Gutenberg 8.4 milestone Jun 11, 2020
This was referenced Jun 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Heading Affects the Headings Block [Block] Paragraph Affects the Paragraph Block [Status] In Progress Tracking issues with work in progress [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

LineHeight styles: Typography panel still visible after disabling
3 participants