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

Audit and better document the useNavigationMenu hook #67828

Open
2 of 6 tasks
afercia opened this issue Dec 11, 2024 · 1 comment
Open
2 of 6 tasks

Audit and better document the useNavigationMenu hook #67828

afercia opened this issue Dec 11, 2024 · 1 comment
Labels
[Package] Block library /packages/block-library [Type] Bug An existing feature does not function as intended

Comments

@afercia
Copy link
Contributor

afercia commented Dec 11, 2024

Description

Splitting this out from #67009 see relevant discussion there together with @ntsekouras and @draganescu

It appears there is some uncertainty about this hook expected behavior and that's in part because it is not documented at all.

From the conversation starting from
#67009 (comment) a few main points emerge:

  • No one seems to recall why passing the Id is optional
    • It is worth noting that some of the returned values are different based on whether the ID is passed or not. For example, when the ID is not passed, isNavigationMenuMissing is assumed to be true which is confusinga not necessarily always true.
  • It's unclear whether menuUnavailable is working as intended or buggy.
  • This test seems to indicate that when the ID is not passed, then useNavigationMenu returns information about all menus while instead, when the ID is passed, it returns information about a specific menu. To my understanding, that's not entirely true as it seems to me this hook returns information about all menus anyways.

Overall, it seems there's some room for improvements. At the very least, some documentation should clarify this hook usage.

Step-by-step reproduction instructions

N/A

Screenshots, screen recording, code snippet

No response

Environment info

No response

Please confirm that you have searched existing issues in the repo.

  • Yes

Please confirm that you have tested with all plugins deactivated except Gutenberg.

  • Yes

Please confirm which theme type you used for testing.

  • Block
  • Classic
  • Hybrid (e.g. classic with theme.json)
  • Not sure
@Vrishabhsk
Copy link
Contributor

Hi @afercia 👋

Let me know your thoughts. Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Package] Block library /packages/block-library [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

No branches or pull requests

2 participants