Account created on 17 January 2007, about 18 years ago
#

Merge Requests

More

Recent comments

🇺🇸United States jastraat

Added a few checks to really make sure there's a label string.

🇺🇸United States jastraat

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)

🇺🇸United States jastraat

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!)

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

Could you check to see if this issue still occurs with the new 3.0 release?

🇺🇸United States jastraat

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 )

🇺🇸United States jastraat

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 -

🇺🇸United States jastraat

Nice solution! This worked well in my testing.
Would someone else like to review the MR before I merge?

🇺🇸United States jastraat

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 ?

🇺🇸United States jastraat

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)

🇺🇸United States jastraat

Many of the fixes from this patch have now been merged in other separate issues; marking as outdated.

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

If using Drupal core's built in settings for replacing spaces and special characters in file names, this is no longer an issue.

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

Made an MR from the patch. Confirmed the error prior to the patch and then confirmed that the patch/MR fixed it.

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

Tested with and without the patch, and the image quality is noticeably improved with this change. Created an MR that reflects the patch.

🇺🇸United States jastraat

jastraat made their first commit to this issue’s fork.

🇺🇸United States jastraat

I created an MR from the patch and added some code cleanup so that the new code quality tests pass.

🇺🇸United States jastraat

jastraat created an issue.

🇺🇸United States jastraat

jastraat made their first commit to this issue’s fork.

🇺🇸United States jastraat

jastraat changed the visibility of the branch 3192704-create-directory to hidden.

🇺🇸United States jastraat

jastraat made their first commit to this issue’s fork.

🇺🇸United States jastraat

The preview generated without an issue for the MR.

🇺🇸United States jastraat

jastraat made their first commit to this issue’s fork.

🇺🇸United States jastraat

I'd like to suggest reopening this issue since the previously linked issue is now needs work and labeled a feature request.

🇺🇸United States jastraat

jastraat made their first commit to this issue’s fork.

🇺🇸United States jastraat

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.

🇺🇸United States jastraat

jastraat made their first commit to this issue’s fork.

🇺🇸United States jastraat

Thank you! This makes sense and the display still looks good in both the default Drupal display, layout builder modal, and iframe modal.

🇺🇸United States jastraat

Alrighty - that makes sense. In that case, the current MR doesn't accomplish that, so if someone who is using language wants to tackle....?

🇺🇸United States jastraat

Updating this to the relevant major version.

🇺🇸United States jastraat

Updating this to the relevant major version.

🇺🇸United States jastraat

Updating this to the relevant major version.

🇺🇸United States jastraat

Updating this to the relevant major version.

🇺🇸United States jastraat

Updating this to the relevant major version.

🇺🇸United States jastraat

I think we need a clearer idea of the desired behavior from a few users who are actively using the multi-lingual capabilities of Drupal.

🇺🇸United States jastraat

Closing; if this is still a problem in the latest release, please reopen with steps to reproduce.

🇺🇸United States jastraat

Closing; please reopen if this is still a problem in the latest release.

🇺🇸United States jastraat

See https://www.drupal.org/project/section_library/issues/3507843 🐛 Adjust post update hooks based on the current version of the module Active for potential fix.

🇺🇸United States jastraat

Found out something super obscure:
"Turns out you shouldn't set data_table in the entity type annotation if translatable is FALSE. Otherwise views associates all of the fields with the data_table, but data_table is never associated with base_table due to translations being disabled."

Removing the data_table property from the entity allows custom fields to appear as options in section library views.

Updated patch attached.

🇺🇸United States jastraat

Updated work around for 10.4.x:

use Drupal\Core\Access\AccessResult;
use Drupal\Core\Form\FormStateInterface;
use Drupal\media_library\MediaLibraryState;

/**
 * Implements hook_field_widget_single_element_WIDGET_TYPE_form_alter().
 * 
 * Add the plugin ID to the media library state when in layout builder.
 */
function MODULE_field_widget_single_element_media_library_widget_form_alter(array &$element, FormStateInterface $form_state, array $context) {
  $route_match = \Drupal::routeMatch();

  if (in_array($route_match->getRouteName(), ['layout_builder.add_block', 'layout_builder.update_block'])) {
    /** @var \Drupal\media_library\MediaLibraryState $state */
    $state = $element['open_button']['#media_library_state'];
    $openerParameters = $state->getOpenerParameters();
    $openerParameters['plugin_id'] = $form_state->getFormObject()->getCurrentComponent()->getPluginId();
    $new_state = MediaLibraryState::create($state->getOpenerId(), $state->getAllowedTypeIds(), $state->getSelectedTypeId(), $state->getAvailableSlots(), $openerParameters);
    $element['open_button']['#media_library_state'] = $new_state;
  }
}

/**
 * Implements hook_ENTITY_TYPE_create_access().
 * 
 * Allow editors with inline block permissions to insert from media library.
 */
function MODULE_block_content_create_access(AccountInterface $account, array $context, $entity_bundle) {
  $route_name = \Drupal::routeMatch()->getRouteName();

  if ($route_name === 'media_library.ui') {
    /** @var \Drupal\media_library\MediaLibraryState $state */
    $state = MediaLibraryState::fromRequest(\Drupal::request());
    $openerParameters = $state->getOpenerParameters();

    // If the plugin ID exists within the opener parameters, we know
    // the media library is being used on the layout builder form.
    if (isset($openerParameters['plugin_id']) && str_starts_with($openerParameters['plugin_id'], 'inline_block')) {

      if ($account->hasPermission('create and edit custom blocks')) {
        return AccessResult::allowed();
      }
    }
  }

  // No opinion.
  return AccessResult::neutral();
}

🇺🇸United States jastraat

Attached patch must be applied to the dev branch of section library.

🇺🇸United States jastraat

When saving the view, we see the following error:

Deprecated function: mb_strtolower(): Passing null to parameter #1 ($string) of type string is deprecated in Drupal\Core\Config\Entity\Query\Condition->compile() (line 39 of core/lib/Drupal/Core/Config/Entity/Query/Condition.php).

This is because the UUID for the configured view is empty. Working on an update hook.

🇺🇸United States jastraat

Thanks! :)

Would you mind rebasing your branch/MR? Unfortunately there are some merge conflicts.

🇺🇸United States jastraat

I tested this and had no issues with cloning and inserting blocks with paragraph fields, media fields, node reference fields, or icon fields. I'd love to know more about the UI icons module; that looks very interesting.

🇺🇸United States jastraat

Alrighty - you can override, but do so at your own risk :)

🇺🇸United States jastraat

This makes sense to me generally. The only instance I'm not sure of is the "ImportSectionFromLibraryController". Is there a use case for overriding this?

🇺🇸United States jastraat

Thank you for this update; choosing and inserting templates works great.

🇺🇸United States jastraat

jastraat made their first commit to this issue’s fork.

Production build 0.71.5 2024