Link Control UI Opens for Last Inserted Block when returning to the Site Editor Right Sidebar Navigation Inserter

[jeryj]

The Problem

It's unclear (without reading the code), why that Link UI would open anytime the component is mounted. It's due to a useEffect hook that opens the UI if:

• There is a lastInsertedBlockId
• The last inserted block is allowed to be inside the navigation block
• The block does not have a URL set

These things are all still true when the component rerenders, so the Link UI shows again when it's mounted.

The Link UI also always shows when the first block in the navigation is missing a url, as the lastInsertedBlockId returns the first block in the array on load. I believe this is because on load it considers all the existing blocks as the last inserted blocks.

How to Solve it

I believe the intention is that the LinkUI shows ONLY once when the link is added. If so, we can set a state to see if that UI has already shown or not, or set the useEffect as such that it only runs if the lastInsertedBlockId has changed.

Test Reproduction Steps

• In the Site Editor Right Sidebar Navigation Inserter:
• Click the + at the bottom to add a new navigation item
• It opens the Link Control UI
• Click Cancel (do not set a link)
• Click to another item or anywhere else to remove the Right Sidebar Navigation Inserter from the view
• Return to the Sidebar Navigation
• The Link UI Will Open

[getdave]

I remember this. Nice work narrowing down the cause.

Can we just have a ref somewhere that tracks whether a block was inserted during whilst the current instance of the List View was mounted. If it wasn't then don't do the link UI thing.

[getdave]

I think the reducer will set the lastBlockInserted state on

case 'INSERT_BLOCKS':
case 'REPLACE_BLOCKS':
case 'REPLACE_INNER_BLOCKS':

I suspect our code dispatches one of those actions which causes the lastInsertedBlock to be all the blocks in the menu.

So we need to distinguish between a user action (i.e. user added this block) and an automattic action.

[getdave]

I the answer to this relies on distinguishing between "this was the last block added" and "this was the block the user just added". We only want to show Link UI in the later case.

~I think in appender we can set some state in the onSelectOrChange and consume that in the effect where the Link UI is triggered. If the local state doesn't exist then we know it's not just been added.~ This won't work for the Link UI that shows in submenus because we won't have access to state from the List View in the Leaf.

I'm exploring another route now.

[getdave]

Related https://github.com/WordPress/gutenberg/issues/50733#top

[getdave]

Research

What's the problem we're trying to solve?

There are two Issues. The one in this Issue and the one in https://github.com/WordPress/gutenberg/issues/50733. These are closely related.

Why do we care about the last inserted block?

This is required in order to display the Link UI for the block that was most recently inserted into the List View. This is because when you click the appender and then select a block, a new block gets inserted into the list view. We then need to grab the clientId of that freshly inserted block and display the Link UI in order that the user can configure the link for that block.

What problems does this cause?

In this Issue, the problem is that the selector getLastInsertedBlocksClientIds may potentially return results long after the user has taken the action which results in the new block being added to the list view.

For example, I click on +, the block gets added to the list view but I click Cancel on the Link UI popover. The block that was added is now the lastInsertedBlock. However, if I then click away from the Nav block and then click back and open the List View Editing the getLastInsertedBlocksClientIds runs again and returns the exact same block. This causes the Link UI to display again which is unexpected because it is out of context for the user - they took the action to add the block a long while ago.

What solution do we need?

We need way to keep track of which was the last block that was inserted by the by the user in this currently rendered list view. That way we can conditionally display the Link UI based on the last block inserted into the current List View rather than the last block inserted anywhere within the editor.

The benefit of this approach would be that a block would only be considered "last inserted" for the currently rendered list view. As soon as the list view unmounts the information about "last inserted" would be discarded, meaning that when the list view is opened again, there will be no "last inserted block" and thus the Link UI will not display.

How is this related to https://github.com/WordPress/gutenberg/issues/50733?

This issue in https://github.com/WordPress/gutenberg/issues/50733 is also related to reliance on lastInsertedBlockClientIds.

Remember that multiple Navigation blocks can contain the same menu. Changes to one block will therefore trigger updates to the inner blocks in another block if it's using the same menu.

THe issue is that when there are x2 Nav block in the same template, the lastInsertedBlockClientIds selector runs once when you insert the block in the list view, but then is immediately re-triggered by the call to replaceInnerBlocks which is part of the automatic syncing of controlled inner blocks in the other Navigation block. This causes lastInsertedBlockClientIds to contain unexpected blocks and thus we get UI bugs.

Again, if the implementation relied on local state to determine the "last inserted" this would not be an issue.

Files

• Blocks are "inserted" by the Inserter (Appender) by the onInsert callback here which calls to insertBlocks or replaceBlocks here .
• When selecting a block from the list the callback (from the above) is called here.

Potential Routes

Passing Values via List View Context

Idea: Pass context along with the insertBlocks or replaceBlocks or replaceInnerBlocks calls which allows us to provide a context in the state as to where the block was inserted from. The create an option on the selector to only select the last inserted block from a given context.

This won't work because the code that controls displaying the Link UI (both for appender and submenu-based) insertion resides in the Nav block code and not within the List View so it doesn't have the necessary Provider to provide the context.

Update: actually it might work if we can pass the appropriate context value through to both renderAdditionalBlockUI and blockSettingsMenu. This value would contain the last block inserted by the list view (derived from its own context) and we can use that to determine whether to show the Link UI in various context's.

Passing "Insertion Context" in the State Action

Idea: Make the last inserted block information local to the list view only, as this is actually what we want. We don't care about "the last inserted block in the editor". We care about "the last block added to the list view component".

This won't work because we only track the last inserted block. It's not a history stack. Therefore even if we could trigger an action that would update the state to say "hey this block(s) was inserted and it was from the offcanvas" it would be immediately lost if another call to insertBlocks happened. This is the problem we're currently experiencing and so adding context to the action objects won't solve it.

Local Tracking State + Passing Context to lastInsertedBlock

Idea: when insertBlock is called from the List View we pass some context (such as location:nav-block-list-view) as per of themetaprop which is already available on theaction. Check that will be stored in thelastInsertedBlockState. Then in the Nav blockmenu-inspector-controls.jscomponent we can call thegetLastInsertedBlocksClientIdsand check whether these blocks have the appropriate meta value (e.g.nav-block-list-view). If so then we stored the clientId in aref` which is local to the block and then check this before showing the LInk UI.

I don't think this will work because the ref will be destroyed when the List View is unmounted and thus we'll loose all context about the last inserted block.