Could you provide steps for recreating this issue?
The code changes look good; would like to see a review from someone actually using layout library though -
I created an MR with my suggestion.
If this looks like a good approach, I could add the same logic in the unpublishing actions.
I rebased on Drupal 11 and moved the new entity update hook to the hook class with the rest. I'd like to throw this back out for review -
Yay!
One big disadvantage of the current approach is that the original draft revision message is replaced with the generic 'Bulk operation publish revision' log message and you lose the information about what author originally made the actual edit.
I'd definitely advocate for retaining the information about who authored the changes and why.
jastraat → created an issue.
Just to confirm, this is still an issue with the latest release of Drupal 11.
This works as designed; the blocks only really exist within the layouts that reference them.
If you feel otherwise, please open a "feature request" instead of a bug.
I created a custom field formatter for the "type" base field on section library template.
This field formatter can then be used in the default section library view, so I updated the default view config.
I did not add an update hook for this since not all sites may want to use the new field formatter in their existing views.
Looks good; created a followup issue to address the code quality issues.
jastraat → created an issue.
This looks great! Created a followup issue to address the code quality issues.
jastraat → created an issue.
jastraat → created an issue.
jastraat → created an issue.
For right now, running GitLab CI only in Drupal 10. Once we add support for Drupal 11, we should remove the following line from the gitlab file:
OPT_IN_TEST_CURRENT: 0
jastraat → created an issue.
jastraat → created an issue.
Thank you for that level of debugging and detail!
Does the attached MR address your issue?
Thank you! Merging the 2.x one since that is the branch actively in development.
Closing as a duplicate of ✨ Update existing library entries Needs review
Thank you!
jastraat → created an issue.
Reducing to normal since this relates to an interaction with custom code.
I will point out that paragraph cloning was broken prior to 🐛 Error after importing section/template that contains blocks with paragraphs without available source content Active for paragraphs in inline blocks where the inline block was saved as part of a layout before creating a section template and then the original source block was deleted.
It looks like the hook is targeting a media entity. How does that interact with the paragraph and block entities? Is there a media reference field on the paragraph?
Added a few checks to really make sure there's a label string.
I don't think strtolower itself is deprecated: https://www.php.net/manual/en/function.strtolower.php
Did you recently update to the 2.0 release? Did you run update.php? (drush updb)
That sounds awesome!
As a followup, we were able to get the new branch of editoria11y to not time out by reworking the mega menu JS (and probably improved site performance along the way!)
Unfortunately since templates can have additional unknown fields, this approach may be really confusing for editors unless there was some additional magic to have editors first pick whether or not they were creating a new template or updating an old and then as a 2nd step if updating, populate the entity form with the existing section template field values. I'm just not sure how feasible this approach is when there could be additional custom fields on the entity.
I love this idea but new features are only being added in the 2.x branch and that branch has changed enormously since this MR was opened. If I get time, I'm going to try creating a new branch/MR based on your work that is compatible with the current 2.x branch. (I already started working on this; there are additional considerations since we only want folks to be able to update templates they have access to edit, sections and templates are now using content entity forms/fields and those two types were merged into the same add form to reduce duplication.
I'm marking this as reviewed since I didn't write the original patch and tested it. This has also been implemented on numerous other modules for the same reason.
Could you check to see if this issue still occurs with the new 3.0 release?
I'm actually no longer seeing an issue with spaces in file names with the latest version of spatie (from 📌 Update module to be compatible with latest spatie/pdf-to-image library and manage the library with composer Active )
I looked into the difficulty of having different title text for the contextual link depending on if it was being shown or hidden. Having dynamic text can be done by extending ContextualLinkDefault and overriding the getTitle() method, however any kind of dynamic title will be complicated by the fact that contextual links are cached in local storage.
The current MR's tests are passing. If having different text for the contextual links is the only hold up, could we consider an alternative label that doesn't change?
Marking as needs review in hopes of getting this merged -
Nice solution! This worked well in my testing.
Would someone else like to review the MR before I merge?
jastraat → made their first commit to this issue’s fork.
Could you try the MR in https://www.drupal.org/project/section_library/issues/3512185 🐛 Existing test are failing after new section library module update Active ?
Could you try the attached MR?
jastraat → made their first commit to this issue’s fork.
I think the solution is to remove all image style config from the install config; we cannot assume that an image style still exists (even if it was included in the default installation of the core image module)
Many of the fixes from this patch have now been merged in other separate issues; marking as outdated.
The image rendering was fixed in https://www.drupal.org/project/pdf_preview_image/issues/2954384 🐛 Poor image quality Needs review
While this removes the need for the wrapper library from imagemagick, it's just a trade of dependencies. https://www.drupal.org/project/pdf_preview_image/issues/3511732 📌 Update module to be compatible with latest spatie/pdf-to-image library and manage the library with composer Active approaches this in a different way by including the required wrapper library in the module's composer file.
If using Drupal core's built in settings for replacing spaces and special characters in file names, this is no longer an issue.
While this could make sense if pdf preview image is only providing a default in the case that a user doesn't upload their own thumbnail, the patch hard codes a field name and in it current format would also prevent a new thumbnail from being generated if the PDF was replaced.
In addition to the automatically generated MR, I updated the format of the dependencies to match the recommended format for Drupal (namespaced in the format {project}:{module}).
Given the warning in the issue description:
"Warning: The 'project-update-bot-only' branch will always be overwritten. Do not work in that branch!"
I created https://git.drupalcode.org/project/pdf_preview_image/-/merge_requests/11 and tested this in a Drupal 11 site successfully.
jastraat → made their first commit to this issue’s fork.
Made an MR from the patch. Confirmed the error prior to the patch and then confirmed that the patch/MR fixed it.
jastraat → created an issue.
This is really cool functionality!
I did get an error when I first tried to preview so I made a tweak.
I think this may also require a cache clear from a post update hook.
Tested with and without the patch, and the image quality is noticeably improved with this change. Created an MR that reflects the patch.
jastraat → made their first commit to this issue’s fork.
I created an MR from the patch and added some code cleanup so that the new code quality tests pass.
jastraat → created an issue.
Looks like this is a duplicate of 🐛 Not creating file if directory is doen't exist RTBC
jastraat → created an issue.
jastraat → made their first commit to this issue’s fork.
jastraat → changed the visibility of the branch 3192704-create-directory to hidden.
jastraat → made their first commit to this issue’s fork.
The preview generated without an issue for the MR.
jastraat → made their first commit to this issue’s fork.
I'd like to suggest reopening this issue since the previously linked issue is now needs work and labeled a feature request.
jastraat → made their first commit to this issue’s fork.
Updated the MR to include logging without injecting the logger. There were pre-existing calls to the logger service for exceptions so this was consistent with the existing code. Also updated the test for the latest Drupal.
jastraat → made their first commit to this issue’s fork.
jastraat → created an issue.
jastraat → made their first commit to this issue’s fork.