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

Transform Page list to navigation block #68402

Open
wants to merge 2 commits into
base: trunk
Choose a base branch
from

Conversation

benazeer-ben
Copy link
Contributor

@benazeer-ben benazeer-ben commented Dec 30, 2024

What?

Fixes #44458

Why?

Currently Page list block has no transform option for navigation block.

How?

Added navigation block transform to Page list block.

Testing Instructions

  • Go to WP admin dashboard.
  • Edit/create any post/page.
  • Create a Page List Block
  • Try to transform it to a navigation block.

Screenshots or screencast

Screenshare.-.2025-02-10.5_17_58.PM.mp4

Copy link

github-actions bot commented Dec 30, 2024

The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: benazeer-ben <[email protected]>
Co-authored-by: t-hamano <[email protected]>
Co-authored-by: carolinan <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@akasunil akasunil added [Type] Enhancement A suggestion for improvement. [Block] Page List Affects the Page List Block labels Jan 6, 2025
@carolinan
Copy link
Contributor

I like this a lot.
I was expecting the page list block to be placed inside the navigation block, and not individual pages.
But I don't feel strongly about it.

@carolinan
Copy link
Contributor

When a page has no title, the page list block displays the text "(no title)" but the transformed list block has the link to the page without a link text. It is not apparent that there is a page link there unless I open the code editor.
The navigation block shows "Add label".

@carolinan carolinan requested a review from t-hamano January 24, 2025 06:11
Copy link
Contributor

@t-hamano t-hamano left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Both transformations are complex, so I recommend splitting this PR first.

The transformation to the List block is probably easier.

The transformation to the Navigation block is quite complex. When transforming from the Page List block to the Navigation block, we would expect to create one new navigation. But this PR creates two new navigations in draft status:

cfe120064d05b026cf8cb195b319fb66.mp4

Perhaps simply running createBlock is not enough.

As of now, I'm still not sure what the best approach is, but I would recommend clarifying the specs for the Navigation block and getting feedback from contributors who know more about it before writing any code.

@benazeer-ben
Copy link
Contributor Author

@t-hamano Thanks for the review! Splitting the PR makes sense, and I'll start by focusing on the transformation to the List block first.

For the Navigation block issue, I see the problem with multiple draft navigations being created. I'll investigate why createBlock is behaving this way and whether additional handling is needed.

Let me know if you have any specific suggestions or references that might help! 🚀

More suggestions are welcome from other contributors too.

@benazeer-ben benazeer-ben changed the title Transform Page list to navigation and list block Transform Page list to navigation block Feb 10, 2025
@benazeer-ben
Copy link
Contributor Author

Hi @carolinan , @t-hamano

As per the above comments, I have split it into separate PRs—one for navigation and another for the list.

This is the PR for transform to navigation block. I have updated description also.

Now, I will work on the above mentioned issues for navigation transform.

@benazeer-ben
Copy link
Contributor Author

Hi @t-hamano

Thanks for your review and suggestion!

When I transform to navigation, only one menu is being created if I am not mistaken. I’m attaching a reference for clarity.

Could you provide more details on how we should proceed with the navigation transformation?

I’d also appreciate any additional suggestions from others.

Screenshare.-.2025-02-10.6_35_54.PM.mp4

@t-hamano
Copy link
Contributor

Could you provide more details on how we should proceed with the navigation transformation?

Strangely, in my environment, multiple menus are created.

Another problem is that after converting a page list to a navigation block, a new navigation is created every time I click the Undo button.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Page List Affects the Page List Block [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Transform page list to navigation block and list block
4 participants