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

Allow setting defaultBlock in navigation block #52610

Closed
wants to merge 1 commit into from

Conversation

sethrubenstein
Copy link
Contributor

@sethrubenstein sethrubenstein commented Jul 13, 2023

What?

Create a defaultBlock attribute and sets innerblocks to reference that if attributes are present in it.

Why?

Solves for #50982

How?

This adds a new defaultBlock attribute and a check for it along with a quick sanity check if it fails it falls back to the DEFAULT_BLOCK constant.

Testing Instructions

  • Open a post or page
  • Copy and paste this block markup into the editor:
<!-- wp:navigation {"defaultBlock":{"name":"core/navigation-link","attributes":{"type":"category","kind":"taxonomy"}}} /-->
  • Click + inside the navigation block.
  • Now category links should be the default when clicking + inside this navigation block.

@scruffian
Copy link
Contributor

It seems like this only works with variations of the navigation link block. Could we also make it work for all blocks that are allowed inside the navigation block?

@sethrubenstein
Copy link
Contributor Author

sethrubenstein commented Jul 13, 2023

@scruffian It defaults to core/navigation-link, it's current default behavior but you could do any block thats supported inside the navigation block, like:

<!-- wp:navigation {"defaultBlock":{"name":"my-custom/nav-block","attributes":{"myValue":"xyz"}}} /-->

@scruffian
Copy link
Contributor

That didn't work in my testing but maybe I was doing it wrong!

@sethrubenstein
Copy link
Contributor Author

You are correct @scruffian, because the allowedBlocks are set as a constant at https://github.com/WordPress/gutenberg/blob/trunk/packages/block-library/src/navigation/constants.js#L5 this will only work for the blocks listed therein. I agree it would be nice to extend that as well, and I believe there is other work underway to standardize around an allowedBlocks attribute #52467, when that work is complete along with this work your use-case will be possible.

CleanShot.2023-07-14.at.12.52.40.mp4

@scruffian
Copy link
Contributor

I was trying it with the search block, which is an allowed block, so I would expect it to work...

@sethrubenstein
Copy link
Contributor Author

Hi @scruffian Just checking back on this, I'm able to get the search block to work, are you passing in something for attributes? In this implementation I'm only passing in the defaultBlock attribute if you give more than just a name, so that the DEFAULT_BLOCK behavior is respected for the time being except for very explicit use cases.

CleanShot.2023-07-25.at.22.05.18-converted.mp4

@skorasaurus skorasaurus added the [Block] Navigation Affects the Navigation Block label Jul 27, 2023
@github-actions
Copy link

⚠️ Type of PR label error

To merge this PR, it requires exactly 1 label indicating the type of PR. Other labels are optional and not being checked here.

  • Type-related labels to choose from: [Type] Accessibility (a11y), [Type] Automated Testing, [Type] Breaking Change, [Type] Bug, [Type] Build Tooling, [Type] Code Quality, [Type] Copy, [Type] Developer Documentation, [Type] Enhancement, [Type] Experimental, [Type] Feature, [Type] New API, [Type] Task, [Type] Performance, [Type] Project Management, [Type] Security, [Type] WP Core Ticket.
  • Labels found: [Block] Navigation.

@draganescu
Copy link
Contributor

One thing to test is that when the navigation block includes other blocks than navigation link the auto insert behavior is turned off. Maybe we should just customize the variation of the navigation link? Like defaultLinkType instead of defautlBlock?

@scruffian
Copy link
Contributor

Ah yes, it wasn't working because I hadn't specified attributes. It does work now.

One thing to test is that when the navigation block includes other blocks than navigation link the auto insert behavior is turned off

This is still the case.

Copy link
Contributor

@scruffian scruffian left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is good to go.

Copy link
Contributor

@draganescu draganescu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this. @sethrubenstein could you consider my idea of the attribute being only for the variation of the navigation link, not for specifying any block? It's just a too finicky for a public API for any block considering we have an allowedBlocks list and that specifying anything other than the navigation link disables the behavior which this PR aims to improve.

What do you think?

@sethrubenstein
Copy link
Contributor Author

@draganescu yeah that works by me. I need to catch up on the auto insertion apis. All I'm really trying to solve for is when you click plus on the nav block to insert a specific variation (in our case the category link variation) of the navigation link block. Let me investigate best way to do that now that there's this insertion api and I'll modify this PR accordingly.

@colorful-tones
Copy link
Member

colorful-tones commented Nov 30, 2023

I'm trying to catch up on all things defaultBlock, but I missed a lot. However, I'm currently trying to get it to work in useInnerBlocksProps, and it will not work at all. Do you know if it only works with the Navigation block? That seems odd, and the README should likely be updated with a pertinent example (if so). 😕

Maybe I'm just missing that defaultBlock does not exist in WordPress core yet because I'm just running WP 6.4.1 and creating a custom block with useInnerBlocksProps and getting nowhere.

@sethrubenstein
Copy link
Contributor Author

@colorful-tones Yeah unfortunately this isn't going to work with Nav block currently. We stabilized the defaultBlock api point in 6.4 so it should be available but I realize now I had the syntax wrong on the docs and need to update that asap. The correct syntax for defaultBlock is {name: 'my/block-name', attributes: { content: 'This block has some predefined content when clicking to insert it into the editor'}} and of course have directInsert set to true as well.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Navigation Affects the Navigation Block
Projects
Development

Successfully merging this pull request may close these issues.

5 participants