Include 'is_active' value in book tree items

Looks good. The patch applies cleanly. Let's see if it passes automated tests.

Patch applied fine to 8.9.10 and added the active class.

Looking at the failed tests it seems to be related to the Classy theme templates that are being copied to other themes. I believe that's being done for some compatibility in Drupal 9, but don't know for sure.

The fix might be as simple as also updating the template files in those themes. I think the doc block in the all the templates files will be needed to updated with the - is_active: TRUE if the link is for the current page. text anyway.

Because there are different core themes for D8 and D9, there might need to be separate patches made for both versions.

Also, I assume a test will need to be added for this new functionality?

Closed #3182994: BookManager generates links that don't get is-active class so added credit here.

Patch applied cleanly, but I didn't see any changes at first because the twig template for the Stable theme isn't included in the patch. Even though that template doesn't add any classes, the docblock at the top should be updated to note that the variables are available for use.

In the project I was using, I called the BookManager service to rebuild the outline when a revision was reverted. That particular route doesn't autoload the node object, causing an error. This might happen in other cases so I added an ugly fix on my patch (I only tested in 8.9.x but should work with other branches too).

Just fixing typo

#9 worked for me once I used the 'is_active' variable to add the 'is-active' class in my theme's twig template:

{% set item_classes = [
  item.is_active ? 'is-active',
] %}

{{ link(item.title, item.url, item.attributes.addClass(item_classes)) }}

Thanks!

The patch in 20 assumes that all routes that bookOutlineRecursive is called from have a node parameter that contains either the node itself or a node id.

On my site I need to show a partial book outline on the node/add page form for child pages. The node/add page doesn't have a node parameter in its route and because of this the patch causes an array_flip() warning. The warning doesn't stop the form from saving, it just adds extra clutter to the logs.

I've attached an update to the patch which adds an extra null check to the bookOutlineRecursive function, to handle the case where the route doesn't have a node parameter at all. When there is no node parameter in the route the value of $node will remain NULL and the is_active flag will not be added to any object in the outline (which makes sense for the node/add page as when that page is called no node exists yet).

Just curious, and maybe this is a dumb question, but why aren't the template changes being added to core/modules/book/templates/book-tree.html.twig instead of the one in classy theme? That is the template that is used if classy isn't enabled (pretty common, i would think).

@scotwith1t it is being added to that template as well, but the default template doesn't really add classes so it is just documented as an available variable. Many themes extend classy to make use of the helpful default classes so it would be very common that classy's template be used ore copied over.

10.1.x update

This extension is being deprecated, see #3376070: [Meta] Tasks to deprecate Book module. It will be removed from core and moved to a contrib project, #3376101: [11.x] [Meta] Tasks to remove Book.

This is now Postponed. The status is set according to two policies. The Remove a core extension and move it to a contributed project and the Extensions approved for removal policies.

This issue may be re-opened if it can be considered critical, If unsure, re-open the issue and ask in a comment.

Needs reroll for contrib only.

Rerolled the patch for contrib module only.

Should be an MR

Created an MR, ready for review.
Thank you!

@kalpanajaiswal I've left some comments and Tests are also failing please check.

Hi, I've addressed the feedback which is mentioned in MR !10.

1. Refactored the $element['is_active'] assignment to use a ternary operator.
2. Replaced the direct \Drupal::routeMatch() call with RouteMatchInterface by injecting it into the BookManager constructor.
3. Updated the service definition to include @current_route_match.

Please review the changes and let me know if any further changes are required.

MR is unmergable. Probably could add a simple assertion somewhere too.