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.

Sign up for GitHub

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

Jump to bottom

FSE: fix Template Part name in Block Navigator #37129

Merged
merged 2 commits into from
Oct 30, 2019
Merged

FSE: fix Template Part name in Block Navigator #37129

merged 2 commits into from
Oct 30, 2019

Conversation

Addison-Stavlo
Copy link
Contributor

Changes proposed in this Pull Request

  • added __experimentalDisplayName and label attribute for header and footer

Before -
Screen Shot 2019-10-28 at 6 34 54 PM

After -
Screen Shot 2019-10-28 at 6 34 10 PM

Testing instructions

Testing with local Gutenberg environment with latest Master branch.

  1. Run this PR
  2. edit a Page
  3. verify that "Header" and "Footer" are titles in the Block Navigation component.

Fixes #36508

@Addison-Stavlo Addison-Stavlo added [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Goal] Full Site Editing labels Oct 28, 2019
@Addison-Stavlo Addison-Stavlo requested review from a team as code owners October 28, 2019 22:41
@Addison-Stavlo Addison-Stavlo self-assigned this Oct 28, 2019
@matticbot
Copy link
Contributor

Test live: https://calypso.live/?branch=fix/template-part-name-in-block-navigation

@noahtallen
Copy link
Contributor

Note for reviewers that this experimental feature might not be on the current dotcom Gutenberg version yet. AFAIK this shouldn't be a problem and the code shouldn't crash, but that's worth a check.

@matticbot
Copy link
Contributor

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

@noahtallen noahtallen mentioned this pull request Oct 29, 2019
Merged
5 tasks
@Copons
Copy link
Contributor

Copons commented Oct 29, 2019

This works as expected! 🎉
I've requested a minor change that though we should tackle before merging.

vindl
vindl approved these changes Oct 30, 2019
View reviewed changes
Copy link
Member

@vindl vindl left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Nice work @Addison-Stavlo! ✨

I verified that this works as expected on self-hosted install.
Also tested it on Horizon with existing FSE site, and made sure it's not causing problems for WP.com where we run previous Gutenberg version that doesn't have this experimental feature. :shipit:

@vindl vindl added [Status] Ready to Merge and removed [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. labels Oct 30, 2019
@Addison-Stavlo Addison-Stavlo merged commit 7c5912a into master Oct 30, 2019
@Addison-Stavlo Addison-Stavlo deleted the fix/template-part-name-in-block-navigation branch October 30, 2019 18:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Copons Copons Copons approved these changes

@vindl vindl vindl approved these changes

Assignees

@Addison-Stavlo Addison-Stavlo

Labels
[Goal] Full Site Editing
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

FSE: template part generic name appears in block inspector
5 participants
@Addison-Stavlo @matticbot @noahtallen @Copons @vindl