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

Automatically update the menu item for the "Top nnn - latest extract" link to match the title #28

Open
2 tasks
bobbingwide opened this issue Oct 12, 2024 · 4 comments
Assignees

Comments

@bobbingwide
Copy link
Owner

The navigation menu item for "Top nnn - latest extract" links to the page https://top-10-wp-plugins.com/top-12-latest-extract/
In the oobit child theme of Twenty Eleven the menu automatically updates to match the title of the page.
It's currently "Top 147 - latest extract" since that's how many plugins have been downloaded more than 10 million times.
As more plugins reach 10M, I update title of the page to match the new number. The slug remains the same.

WIth the oobit-2025 theme the menu item doesn't update automatically.
This is a minor but annoying problem.

Requirement

  • Selected menu items to update automatically to display the current posts title.

Proposed solution

  • Attempt to find a solution that doesn't require any code changes
  • If that's not possible then extend the wp-top12 logic to implement render_block_data for navigation menu items.
@bobbingwide
Copy link
Owner Author

bobbingwide commented Oct 12, 2024

In the current implementation the navigation menu is stored as a reference to post 1326.

<!-- wp:navigation {"ref":1326,"fontSize":"small","layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right"}} /--></div>

In order to find which blocks are in the navigation block you have to use the Site Editor.

  • From the Design tab choose Navigation
  • On the required menu - in my case it's called Primary - choose vertical ellipsis > Edit
  • Switch to the code editor.

Attempt to find a solution that doesn't require any code changes

I failed to do this. I tried blanking out the label but that left a useless menu item.
I'm therefor looking at the alternative.

I've duplicated the link to page ID 332. The original link is

<!-- wp:navigation-link {"label":"Top 147 – latest extract","type":"page","id":332,"url":"https://s.b/bwcom/top-10-wp-plugins/top-12-latest-extract/","kind":"post-type"} /-->

The new link is similar

<!-- wp:navigation-link {"label":"Top nnn - latest extract","type":"page","id":332,"url":"https://s.b/bwcom/top-10-wp-plugins/top-12-latest-extract/","kind":"post-type","className":"_title"} /-->
  • The label is deliberately set to "Top nnn - latest extract" so I can see whether or not the override is working
  • The className attribute is set to _title

The override code will detect this className value and use it to replace the label attr with the post title of the referenced post id.

@bobbingwide
Copy link
Owner Author

bobbingwide commented Oct 13, 2024

The method that I wanted to use, which was to extend the logic already developed for #27, doesn't work in the front end since my navigation block is defined using the ref attr.

<!-- wp:navigation {"ref":1326,"fontSize":"small","layout":{"type":"flex","setCascadingProperties":true,"justifyContent":"right"}} /-->

When this is used the core/navigation block doesn't quite render the inner blocks in the same way as other blocks.

C:\apache\htdocs\bwcom\wp-includes\blocks\navigation.php(152:0) WP_Navigation_Block_Renderer::get_markup_for_inner_block(15) 157 0 2024-10-13T10:19:15+00:00 2.180407 0.000395 cf! 9334 48 1489<>0 6291456/6291456 256M F=555 inner_block WP_Block Object

    [parsed_block] => Array

        [blockName] => (string) "core/navigation-link"
        [attrs] => Array

            [label] => (string) "Top nnn - latest extract"
            [type] => (string) "page"
            [id] => (integer) 332
            [url] => (string) "https://s.b/bwcom/top-10-wp-plugins/top-12-latest-extract/"
            [kind] => (string) "post-type"
            [className] => (string) "_title"

        [innerBlocks] => Array
        [innerHTML] => (string) ""
        [innerContent] => Array

which, for the block I want to override, results in

<li class="has-small-font-size wp-block-navigation-item _title wp-block-navigation-link"><a class="wp-block-navigation-item__content"  href="https://s.b/bwcom/top-10-wp-plugins/top-12-latest-extract/"><span class="wp-block-navigation-item__label">Top nnn - latest extract</span></a></li>

Note: The value for the className attr ( _title) is not part of the output.

Each navigation-link is rendered by WP_Navigation_Block_Renderer::get_markup_for_inner_block().
The logic invokes the callback method to render the navigation-link block, but the render_block_data filter isn't invoked
beforehand.

We therefore have to add a filter function to adjust the output during render_block_core/navigation-link.

I already had to use this technique for the wp-countup-js block, but for a different reason.

@bobbingwide
Copy link
Owner Author

bobbingwide commented Oct 13, 2024

I already had to use this technique for the wp-countup-js block, but for a different reason.

In #27 I proposed to use render_block_data to replace relevant attributes with the latest values. This worked for my oik-sb/chart block since the code is dynamically generated.

But it didn't work for the wp-countup-js block.
Upon further investigation I found the explanation.

  • The wp-countup-js block doesn't have a callback function,
  • so the content is rendered directly from $this->inner_content.
  • The inner_content variable is populated from the $block array parameter during __construct().
  • It doesn't get updated after render_block_data.
  • Therefore, changing the values of innerHtml and innerContent won't take affect during rendering.
  • But the updated values can be applied in the post rendering filter eg render_block_roelmagdaleno/wp-countup-js.

It's all a bit bloody messy if you ask me!

I wrote to the #core-editor channel in Making WordPress Slack

Hi, I'm looking at the render_block_data filter and have determined that it's (almost) pointless changing the values of innerHTML or innerContent in the returned $parsed_block array since the changes are not immediately reflected in the WP_Block's $inner_html or $inner_content properties. I was only able to complete the block extension logic that I'd planned by also implementing a filter function for the post rendering filter.... eg render_block_roelmagdaleno/wp-countup-js. Does anyone have any opinions on this limitation? Is this a known issue?

@bobbingwide
Copy link
Owner Author

I wrote to the #core-editor channel in Making WordPress Slack

I got a reply from @dlh01 saying look at WordPress/wordpress-develop#7522
which is a response to TRAC ticket https://core.trac.wordpress.org/ticket/62046

This PR and ticket primarily refers to the render_block_context filter but the solution also seems to take into account changes to the $parsed_block made by render_block_data filter functions.

I don't think there are any new tests that implement filter functions for render_block_data though.
And I'm not in a position to (easily) test the code changes.

I should probably also test whether or not the problem I discovered is due to the fact that the wp-countup-js block is being processed as an inner block of a group. Does the same problem occur when it's a top level block?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant