Use page list instead of placeholder as fallback #42735
Conversation
|
Size Change: +501 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
draganescu
requested review from
getdave
and removed request for
tellthemachines
|
Whatever we do here has to include changes from #42182 in order to ensure they work together. |
draganescu
force-pushed
the
try/page-list-fallback
branch
from
3807343
to
c4ce023
Compare
|
Some things I noticed, perhaps not related to this change or necessary to fix in this PR
I found this too: #42856 |
|
@scruffian the template part seems to pick up the changes for me: template-part-modified.mp4 |
There was a problem hiding this comment.
Thank you for working on this tricky problem.
Screen.Capture.on.2022-08-01.at.16-19-18.mov
On first pass I wanted to stress test this PR by ignoring the instructions and just coming to it with no expectations.
❌ I opened the Post Editor and added a Nav block and I ended up in an unresolvable state with no way to select, create or add items to my menu. I had x4 Navigation Menu posts in the database when I did this.
✅ When I manually deleted all my Navigation Menus and then added a Nav block I was presented with a Page List.
✅ When I then saved that Navigation and then opened a new Post and added a new Navigation block, the block automatically selected the first Navigation menu.
❌ When I tried to create a new Navigation Menu via the Create new menu item nothing happened and I went back to an unresolvable state.
At this point I'd say there a few side effects and problems that we need to iron out. Given the complexity of the scope of the block's interactions, I often find that adding e2e tests helps me to keep track of avoiding regressions and also exposing edge cases. You might want to explore that.
PS: "unresolvable state" looks like this
draganescu
force-pushed
the
try/page-list-fallback
branch
from
b781d7c
to
5d91de2
Compare
|
If I have
This feel unintuitive, but it also does make sense when you understand why.... |
scruffian
force-pushed
the
update/respect-uncontrolled-inner-blocks-on-nav-block-in-editor
branch
from
ff5669f
to
4c52005
Compare
draganescu
force-pushed
the
try/page-list-fallback
branch
from
552da29
to
5d1fd0c
Compare
There was a problem hiding this comment.
I tested this principally by creating x2 menus in Empty Theme and then switching to TT2.
- Initially it rendered the most recent menu (both Editor and FoS)
- I then manually deleted 1 of the x2 menus.
- I then reloaded. It rendered the single menu (both Editor and FoS)
- I then manually deleted the remaining menu.
- It then rendered the Page List (both Editor and FoS).
This seems good. Will continue testing.
Missing e2e tests
We'll want/need to update the e2e tests to match the new functionality. I know @talldan is rewriting these in Playwright so that's going to cause some pain.
Issues
I did notice the following when testing in the Post editor:
Creating a Menu then deleting leaves empty block on front of site
I created a Menu and saved it. Then I deleted it and when back to the Editor. I saw the "your block is not longer available message" but on the front of the site there was no fallback displayed.
I created a new menu and saw permissions warning
Update: I've tried to fix this over here.
I deleted a menu and then refreshed the post. Editor showed me the msg to warn about deleted menu but when I click on the message it shows me a permissions warning.
Screen.Capture.on.2022-08-03.at.14-40-14.mp4
Empty Navigation renders empty on front end
I created a new menu and left it empty. Then I saved. I switched to the front of the site and I saw an empty menu. This is expected but the <nav> element was still rendered. Should it not rendering no block at all?
draganescu
force-pushed
the
try/page-list-fallback
branch
from
efcdb1e
to
6072b40
Compare
…eated block menu. also added a missing await in the test classic menu selecting function
|
🚀 |
getdave
added
the
Needs Dev Note
|
@draganescu I think we need to ensure we add a dev note for this one. The change I wrote about here which got merged into this PR means that any Themes which include a Page List in their bundled template parts will cause their users to effectively "opt out" of the default fallback experience. It's actually quite a significant change as many Themes have taken to including Page List in their Nav block inner blocks. |
Navigation Block Fallback Behavior in WP 6.1 Dev NoteIn WordPress 6.1 the navigation block will have a new fallback behavior. What is the fallback behavior? When a theme uses a navigation block in a template part , the aim is to hint at where the navigation should visually be located, for the theme's UI to be consistent, usable and good looking. However, themes can't know beforehand what menus the site has, how many pages, what they're called and so on. For this reason, themes include a navigation block in template parts, but without knowing what content they'll show. The fallback behavior is the small heuristics in the navigation block which tries to determine what the block should display, by default, when a user activates a theme. What is the new fall back behavior? Starting with WordPress 6.1, theme developers and authors can lean on the following fallback behavior of the block: If the navigation block has inner blocks, it will honor them and display that. If it is empty however, then it will initialize the fallback behavior. The fallback behavior (in both the editor and the front of the site) is:
The key changes can be summarised as follows:
Consistency: previously the fallback behavior was not consistent between the front of the site and the editor. If a theme used an empty navigation block it would display a list of pages on the front, and an empty block in the editor. Now the behavior is consistent between both meaning that the editor mirrors what users see on the front. Defaulting to page list: previously, themes which wanted to default to a page list in the editor usually included a page list inner block within the Navigation block. With this update this is no longer necessary, as the block, if empty, will automatically have consistent front and editor behavior, eventually defaulting to a page list. Selecting the most recent block menu: this part of the fallback behavior is new, in the event a site has multiple block menus, an empty navigation block will display the most recent one. Notes Display only. The fallback behavior only affects what the empty navigation block will display. Unless the user edits the navigation block's default fallback, adding a link, changing a label, converting a page list block to a list of links or selecting another menu, the markup of the template part is not changed. Default content is still honored. There is no change to how navigation blocks with inner blocks from theme markup behave. Themes still include inner blocks in the markup in the event they want to showcase a specific situation, for instance a small three links menu, pointing to #, with some restriction on length of link labels - this will continue to work, just like before, rendering the uncontrolled inner blocks both on the front and in the editor. Props Dave Smith (@getdave ) for editing and technical review. |
Closes #42563
selectClassicMenuto pull the block toolbar instead of placeholderWhat?
This makes the navigation block default to a page list in the editor when loading an empty navigation block (one that has no uncontrolled blocks nor any ref).
Why?
We do this so that the editor respects the fallback of the front end, which is a page list.
How?
We use a similar technique to the one used in the pattern block which does not add the pattern before it is edited.
Testing Instructions
Screenshots or screencast
N/A