Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Navigation Overlay customisation via Template Parts #55657
base: trunk
Are you sure you want to change the base?
Navigation Overlay customisation via Template Parts #55657
Changes from all commits
a4ccff6
a0b5571
9049cb0
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Check failure on line 81 in lib/experimental/blocks.php
GitHub Actions / PHP coding standards
Check warning on line 84 in lib/experimental/blocks.php
GitHub Actions / PHP coding standards
Check warning on line 85 in lib/experimental/blocks.php
GitHub Actions / PHP coding standards
Check failure on line 95 in lib/experimental/blocks.php
GitHub Actions / PHP coding standards
Check failure on line 103 in lib/experimental/blocks.php
GitHub Actions / PHP coding standards
Check warning on line 1 in lib/experimental/navigation-overlay.php
GitHub Actions / PHP coding standards
Check warning on line 6 in lib/experimental/navigation-overlay.php
GitHub Actions / PHP coding standards
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Router shouldn't be a dependency of the block library package as not every editor has one.
@glendaviesnz added a way to handle navigation in #57036 via an editor setting that will work across the post and site editors.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the pointer Dan 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There may be some modifications coming to that approach now as we need to extend it a little probably in order to get rid of the template-only mode, will hopefully get that sorted today.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this mode you speak of?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
https://github.com/WordPress/gutenberg/blob/trunk/packages/editor/src/store/actions.js#L579
It was added temporarily as part of the editor unification work, but we are now in the process of pulling it out again. Context being we wanted a way to edit a synced pattern in isolation in the post editor, so we looked at adding a
pattern-only
mode, but it was then decided that this was not very scalable as we didn't want modes for lots of different entities.It was decided that a better approach would be to change the entity that was passed into the editor provider, and we added a
getPostLinkProps
editor setting which is a callback that returns anonClick
event andhref
that changes the current entity in the editor.We initially just used this for editing synced patterns in the post editor but I am now looking at doing the same for editing templates from the post editor, which currently uses the
template-only
mode. When working through this it seems our initial implementation doesn't extend quite so well for the wider use case, so I have a suggested refactoring of this here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't worry - we can make a separate PR for this block if/when we decide to merge this branch.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@WordPress/interactivity-api This block is going to effectively be the same as the existing button which toggles the overlay. It's required because when editing the overlay within the editor, users need to be able to:
I'd really appreciate any advice/help on how best to transfer the interactivity directives over the this block 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been doing so with the
WP_HTML_Tag_Processor
quite successfully so far.We basically use the
WP_HTML_Tag_Processor
on the render callback of the block, search for the inner block, and add the necessary directives. Something like this:There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There are more routes available, so this doesn't work. Example repro:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, we should still have the Back button navigation in the central header area as on other templates.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's turned into a wider problem. I agree we need it but it will need to be a seperate PR.
Check failure on line 6 in packages/block-library/src/navigation-overlay-close/index.php
GitHub Actions / PHP coding standards
Check failure on line 7 in packages/block-library/src/navigation-overlay-close/index.php
GitHub Actions / PHP coding standards
Check failure on line 8 in packages/block-library/src/navigation-overlay-close/index.php
GitHub Actions / PHP coding standards
Check failure on line 11 in packages/block-library/src/navigation-overlay-close/index.php
GitHub Actions / PHP coding standards
Check failure on line 21 in packages/block-library/src/navigation-overlay-close/index.php
GitHub Actions / PHP coding standards
Check failure on line 26 in packages/block-library/src/navigation-overlay-close/index.php
GitHub Actions / PHP coding standards
Check failure on line 31 in packages/block-library/src/navigation-overlay-close/index.php
GitHub Actions / PHP coding standards