Account created on 27 October 2021, over 3 years ago
#

Merge Requests

More

Recent comments

I had this same issue. patch works well, looks like a clean solution. Created MR and moving to NR. I think if we want to do some js work that should be done in a separate issue.

mdranove changed the visibility of the branch 2825730-issue-with-selections to active.

mdranove changed the visibility of the branch 2825730-issue-with-selections to hidden.

Added issues for the modules listed above.

Tested on drupal 11 install. Placed a book navigation block. With both show_top_item and use_top_level_title enabled, both configurations work as described.

Pushed up a commit to shorten the description for show_top_item. Moving to RTBC.

Reviewed on drupal 11.1.7, book 2.0. /book path only viewable to users with the permission. Update hook applies permission to any user with the access content permission.

I reviewed the docs, saw this

Alternatively, reorder nodes on the book listing. Choose Structure >> Books to see a list of the existing books. Click on the Edit Order And Titles button for the book you want to edit, drag the nodes into the structure and order them, click on Save Book Pages to save the changes.

That being said, I went ahead and

  • Added info about the new setting to change the sorting method on the book settings tab.
  • Fixed a few typos.
  • Added information about the book_olivero submodule.
  • Added info about the new setting to change the sorting method on the book settings tab.
  • Fixed a few typos.
  • Added information about the book_olivero submodule.

I followed the repro steps on a fresh drupal install with core 10.4.7. Then installed book module, did not create any book nodes. Upgraded to core 11.1.7. Could not reproduce. Is there another step here?

@miksha, good point, you're right, that being said, in #37 I get a 403 error when I try it. I do think that might be the way to go, although I wonder if there isn't a way to leverage the "include query parameters" config that already exists.

I was having trouble getting the functionality in the patch #37 to work, but also, I still dont think this should be a hook. I like how the current implementation attaches the library only when the data_export display is attached to the view. The MR I opened keeps that functionality, but allows the user to separately decide if they also want the icon to display.

Hmm yeah and the more I think about it a hook in general doesnt seem like a great solution.

To recap though, all of this is because the library is attached with the icon, but a lot of sites don't want to use the icon because it's not that easy to style/position. I think what should be done is keeping the config option to attach the export to a view, but separating it from the icon by creating a separate option to attach an icon to the view. This way, you can choose to attach the library to a specific view, without needing to necessarily use the icon provided by the module, though that option is still available.

I dont think the approach in the patches are exactly right. The page_attachments_alter approach in #24 would attach the library on every page on the site. #29/31 relies on checking the status messages array which seems unnecessary and prone to breaking as explained in #33.

This patch here I believe is appropriate. It will attach the library on any view that uses the data_export display.

Configuration section of Readme currently refers to https://www.drupal.org/docs/contributed-modules/book/overview for more information.

This page contains the following paragraph added January 21st,

If the navigation links are not displaying on a node, make sure the content type's Book navigation field is above the Disabled line in the Manage Display tab. This is located at: Structure >> Content Types >> Books >> Manage display.

Don't think there is further work needed.

Im not entirely sure the current approach in MR 5004 is the right approach.

Started a new branch. I think it makes sense to just set this to not be required if it's the field config form. Only issue is there's logic setting the display to none by default for the summary default value. Think only thing left is to show that in JS on click of the checkbox. Then add a test.

I updated this for 2.1.x. The original MR is good. Just tweaked it to add a bit from patch #9. Also while testing was getting an exception in src/Plugin/QueueWorker/LinkStatusHandle.php, so I added a bit of logic there to try and make sure we aren't merging any breaking changes.

Please review.

Updated pattern, added test based on feedback. Seeing expected result when tags are placed inside of . Moving to NR.

Bumping to major as this seems like a fairly big issue, flagged in multiple threads.

I checked it out. Good there's no exception now, but error message should explain the issue, not just "0 is invalid". E.g say it needs to be an integer. Also, users should be allowed to leave the field empty, that shouldnt generate an error IMO.

But all this is second of importance to doing something about https://www.drupal.org/project/linkchecker/issues/3065056 🐛 "Don't treat these response codes as errors" not used Active . Until that's resolved, this configuration doesn't do anything. It's in NR right now. Can someone review?

Tested and fixed merge conflict in MR !60. Added an update hook and did a little cleanup on the views config, fixed phpstan error.

Id say it's pretty RTBC at this point, also closes at least two dupes.

Production build 0.71.5 2024