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 template part area variation matching #49500

Merged
merged 2 commits into from
May 2, 2023

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Mar 31, 2023

What?

Fixes #45935

Ensures template part semantic variations are shown correctly.

Why?

This was a regression

How?

Template parts have two different categories of variations. Area variations and instance variations.

The problem was that both area and instance variations can have the same name ('header' or 'footer'), and the name for block variations should be unique. When registering the variations, the first 'header' area variation was being registered correctly, but the 'header' instance variation failed to be registered due to an existing variation with that name.

The variation match function then failed because it prioritises matching by an instance's slug (since #45672), and the 'header' instance variation failed to be registered.

The fix is to add unique names for area template part variations and instance template part variations, so they can peacefully co-exist.

After feedback, this PR now also avoids showing the area variation in the inserter when a matching instance variation exists.

Testing Instructions

  1. Open the site editor
  2. Select a header or footer template part
  3. Ensure it has the 'header' or 'footer' semantic area set (check the Advanced panel in the Inspector)
  4. Observe the icon, and block name in the Inspector

Screenshots or screencast

Screen Shot 2023-03-31 at 3 01 05 pm

@talldan talldan added [Type] Regression Related to a regression in the latest release [Block] Template Part Affects the Template Parts Block labels Mar 31, 2023
@talldan talldan requested a review from jameskoster March 31, 2023 07:10
@talldan talldan requested a review from ajitbohra as a code owner March 31, 2023 07:10
@talldan talldan self-assigned this Mar 31, 2023
@talldan
Copy link
Contributor Author

talldan commented Mar 31, 2023

@jameskoster Unfortunately this does introduce a separate issue. Now that the variations work correctly this situation can occur in the block inserter:
Screen Shot 2023-03-31 at 2 56 00 pm

Here you have 'Header' and 'Footer' variations for inserting new empty template parts for those areas. There are also 'Header' and 'Footer' template parts for inserting existing template parts with those names.

Some possibilities for resolving this:

  • Hide the inserter items for new empty Header/Footer template parts when an existing template part exists (assuming the user is more likely to insert the existing template part.
  • Differentiate the 'instance' template part variations from the others by using different icons or moving them to the 'reusable' tab.

@github-actions
Copy link

github-actions bot commented Mar 31, 2023

Flaky tests detected in 7f785ba.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4616504085
📝 Reported issues:

@jameskoster
Copy link
Contributor

Thanks for the PR 🙏

Good spot with that duplication issue in the Inserter.

or moving them to the 'reusable' tab.

I suspect something like this may happen naturally as we explore #48458 in more detail, but that's a little way off.

Hide the inserter items for new empty Header/Footer template parts when an existing template part exists (assuming the user is more likely to insert the existing template part.

This seems like the better short-term approach to me.

Otherwise this is working as expected:

Before
Screenshot 2023-03-31 at 15 49 30

After
Screenshot 2023-03-31 at 15 49 21

@jameskoster jameskoster requested a review from jasmussen March 31, 2023 14:50
@jasmussen
Copy link
Contributor

Just referencing #35453, which may need to be closed as wontfix as a result of this one. Which is probably fine, for simplicity, it seems okay to move this forward. The purple has helped a lot in making the reusable pieces feel as such.

@ntsekouras
Copy link
Contributor

Differentiate the 'instance' template part variations from the others by using different icons

I think that's a better approach, given the insertion flow for these variations with targeted patterns etc..

@jameskoster
Copy link
Contributor

I think that gets a bit tricky. A user is unlikely to understand the difference between two blocks called "Header", even if they have different icons.

I'd also say that once you've created a header or two, it's much more likely you'd want to insert one of those directly, rather than inserting the empty instance and selecting from there. IE the value of the empty Header template part goes down quite a bit.

The only reason I can think to insert the empty instances would be to create a new template part from scratch. If this is the route we want to go down, then I think it might be better to repurpose those blocks to be more about creating new template parts. This was proposed a little while ago in #31746. The designs are a bit out-of-date, but it communicates the idea.

@talldan talldan force-pushed the fix/template-part-variation-matching branch from f23421a to 7f785ba Compare April 5, 2023 08:28
@talldan
Copy link
Contributor Author

talldan commented Apr 5, 2023

I've updated the PR to hide the inserter variations for areas when a matching instance is found for that area.

This was proposed a little while ago in #31746. The designs are a bit out-of-date, but it communicates the idea.

Oh yeah, forgot about that one, I never did get it working in a satisfactory way. I'll remove my assignment myself for now, but it'd be worth taking a look at it with fresh eyes.

@talldan talldan merged commit 1fde765 into trunk May 2, 2023
@talldan talldan deleted the fix/template-part-variation-matching branch May 2, 2023 08:18
@github-actions github-actions bot added this to the Gutenberg 15.8 milestone May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Template Part Affects the Template Parts Block [Type] Regression Related to a regression in the latest release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Template Part semantic areas are no longer recognised
4 participants