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

Unify implementations of converting WP menu items to Nav Link blocks between nav editor and block #31599

Closed
getdave opened this issue May 7, 2021 · 1 comment · Fixed by #31606
Assignees
Labels
[Block] Navigation Affects the Navigation Block

Comments

@getdave
Copy link
Contributor

getdave commented May 7, 2021

What problem does this address?

Currently we have two locations within the codebase that convert WP Nav Menu items into core/navigation-link blocks:

  1. Nav block - when creating a block from existing menu items.
  2. Nav editor - when rendering a menu within the editor.

These do (almost!) exactly the same thing but differ in their implementation.

What is your proposed solution?

I am proposing that we unify under a single well-tested implementation. Once the implementation is in place, we can (for now) then duplicate it between block and editor.

In the longer term, we can then remove the duplication by (hopefully) placing the conversion code into the proposed @wordpress/menus package.

Note I would do this in the PR that proposes the package, but it would cause the PR to become overly complex and large. I'd prefer to refactor in isolation from the package PR and then rebase that PR when we are confident we have a stable implementation across both codebases.

@getdave
Copy link
Contributor Author

getdave commented May 13, 2021

Closed by #31606

@getdave getdave closed this as completed May 13, 2021
@getdave getdave linked a pull request May 13, 2021 that will close this issue
12 tasks
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
None yet
Development

Successfully merging a pull request may close this issue.

1 participant