Transform Nav Links with children into Submenus #34831
Conversation
tellthemachines
requested review from
getdave,
georgeh,
talldan and
draganescu
|
Size Change: -269 B (0%) Total Size: 1.06 MB
ℹ️ View Unchanged
|
|
What if... instead of transforming the block right away, we displayed a static markup rendered on the backend with an overlay saying "This block is now deprecated, click here to migrate to the new version?" This way the markup wouldn't change unexpectedly, we wouldn't activate the "Save" button on load, and we could still remove the obsolete logic. |
|
Yes @adamziel 's suggestion makes a lot of sense. This way we can remove the code and also not cause an unexpected behavior. |
We did something similar with the Gallery block #34606. We are not automatically deprecation/migrating the old gallery blocks with the initial release, but wanted to give users a way to manually migrate - in this case a button in the toolbar launches the overlay which explains the update process. |
I recently realised this is the expected behaviour for block deprecations, so I shouldn't have listed it as a downside. Apologies for inducing folks in error with my inaccurate PR description 😅 - I'll edit it to remove that line.
It couldn't be an option though, otherwise we wouldn't be able to remove the extra logic. Authors would only have the choice of accepting the changes or leaving the post or template altogether, they wouldn't be able to edit it and preserve the deprecated markup. I'm not sure this would be a great experience tbh, especially because the changes are barely noticeable in the interface. The behaviour for creating submenus remains exactly the same, and any migrated submenus will look exactly the same as they did before.
In the Gallery case it's more justified though, both because the changes are super evident, and because the block has been in core for ages. The Navigation block is still experimental in the plugin only, so the risk of breaking things for anyone is very minor. |
Ooh interesting! I'm want to learn more, how did you find out?
Couldn't we display the pre-rendered thing without offering any interactions?
My thinking is they would still be able to edit the rest of the post, just not this specific part of it. On save, we'd reuse the same "raw" markup as we received from the API.
Wait, would they? My thinking was this could break the display because the markup changes but any custom CSS does not.
If the structure remains the same or similar enough, the implicit replace thing could work. |
|
They look fairly similar! The structure is the same, and the only backwards-incompatible differences I spotted were in class names:
A defensive approach here would involve re-adding the old class names to their corresponding elements. I'm not sure if that would break anything though. What are your thoughts on that @tellthemachines ? Old markup: <ul class="wp-block-navigation__container">
<li class=" wp-block-navigation-item has-child wp-block-navigation-link">
<a class="wp-block-navigation-link__content wp-block-navigation-item__content" href="/about">
<span class="wp-block-navigation-link__label">About</span>
<span class="wp-block-navigation-link__submenu-icon wp-block-navigation__submenu-icon">
<svg xmlns="http://www.w3.org/2000/svg" width="12" height="12" viewBox="0 0 12 12" fill="none" role="img" aria-hidden="true" focusable="false">
<path d="M1.50002 4L6.00002 8L10.5 4" stroke-width="1.5"></path>
</svg>
</span>
</a>
<ul class="wp-block-navigation-link__container wp-block-navigation__submenu-container">
<li class=" wp-block-navigation-item wp-block-navigation-link"><a class="wp-block-navigation-link__content wp-block-navigation-item__content" href="/privacy-policy"><span class="wp-block-navigation-link__label">Privacy policy</span></a></li>
</ul>
</li>
<li class=" wp-block-navigation-item wp-block-navigation-link"><a class="wp-block-navigation-link__content wp-block-navigation-item__content" href="/blog"><span class="wp-block-navigation-link__label">Blog</span></a></li>
</ul>New markup: <ul class="wp-block-navigation__container">
<li class=" wp-block-navigation-item has-child open-on-hover-click wp-block-navigation-submenu">
<a class="wp-block-navigation-item__content" href="/about">About</a>
<button class="wp-block-navigation__submenu-icon wp-block-navigation-submenu__toggle" aria-expanded="false">
<svg xmlns="http://www.w3.org/2000/svg" width="12" height="12" viewBox="0 0 12 12" fill="none" role="img" aria-hidden="true" focusable="false">
<path d="M1.50002 4L6.00002 8L10.5 4" stroke-width="1.5"></path>
</svg>
</button>
<ul class="wp-block-navigation__submenu-container">
<li class=" wp-block-navigation-item wp-block-navigation-link"><a class="wp-block-navigation-link__content wp-block-navigation-item__content" href="/privacy-policy"><span class="wp-block-navigation-link__label">Privacy Policy</span></a></li>
</ul>
</li>
<li class=" wp-block-navigation-item wp-block-navigation-link"><a class="wp-block-navigation-link__content wp-block-navigation-item__content" href="/blog"><span class="wp-block-navigation-link__label">Blog</span></a></li>
</ul> |
Happened to find this convo on Slack a few days ago. Then I went and experimented with a few blocks that have deprecations, by creating the blocks in an older version of WP and then opening the post with a newer version, and unsaved changes do appear.
I don't know if having a switch preventing certain blocks from being editable/saved would be useful in any other situation, but it seems overkill for this particular one 😅
Ah, that discrepancy won't be a problem because, as of #34171, all the styles are attached to the generic classes shared by both sets of markup. It was actually my intention to clean up some of those extra classnames that are no longer needed, but forgot about it. I'll go create an issue for it now 🗒️ Again, the Navigation block is still experimental, so these types of changes are expected. Once it lands in core we'll have to be more cautious. |
tellthemachines
added
[Block] Navigation Link
Description
With #33775, navigation submenus are now rendered as Submenu blocks, instead of being nested inside Navigation Link blocks.
Although Navigation is not yet in core, it's in Gutenberg, so there are likely instances of it in use out there. This means we need to consider how to deprecate submenus in Navigation Links and migrate them safely to Submenu blocks.
Because we're switching from one type of block to another, a standard block deprecation/migration won't work here.
This PR tries transforming Navigation Link blocks with children into Submenu blocks as soon as they load in the editor.
Advantages:
Downsides:Opening a post with legacy Navigation Links in it will show unsaved changes straightaway.(Edit: this is expected with deprecations)Other thoughts:
All thoughts and feedback welcome!
How has this been tested?
Screenshots
Types of changes
Bug fix (non-breaking change which fixes an issue)
(Marking this as bug fix mostly because it patches the issue of using the new open on click functionality with Navigation blocks created prior to #33775, though it's not really a bug as much as a known breaking change 🤔)
Checklist:
*.native.jsfiles for terms that need renaming or removal).