🇺🇸United States @quicksketch

Thanks @sthomen! We have used your MR on our project and it has fixed all the issues with svg_image. However, the PR is full of white-space issues. It's using a mix of tabs and two spaces all over the place. Looking at the diff (https://git.drupalcode.org/project/svg_image/-/merge_requests/29/diffs) you can spot the lines that have tabs by the "over indentation", since they render as 4 spaces in GitLab.

The changes work overall, and includes new test coverage.

I couldn't get #40 working. Here's an updated version that works on Drupal 10/11.

Having now spent a couple of weeks regularly running into this error of "Too many requests" from Google's API, and having failed to request a rate limit increase from Google themselves, I took a hand at resolving this issue by extending this module's plugin class. Having put it together, I think solving the problem here through a configuration option would be the best solution.

I would suggest that a YAML configuration option be added like so:

  # Specify a cache lifetime in seconds. This will persist a local cache of
  # remote spreadsheet API request responses. This can be used to lower
  # the number of API requests to fit within rate limits and avoid a
  # "429 Too Many Requests" response from Google. Omit or set to 0 to
  # disable the cache.
  cache_lifetime: 10

On further investigation, this might be an issue with Migrate Tools module. When running through the API, only a single item is imported per batch request. But running through Drush seems to circumvent the problem. Note when running through Drush, When using Drush, only 2 HTTP requests are made total.

quicksketch → created an issue.

quicksketch → created an issue.

(Sorry wrong screenshot attached, fixing previous comment)

🐛 Refactor to use libraries and compatibility with Big Pipe Needs work is definitely a big improvement on the overall situation.

For our site, where we are using the "Dropdown" widget, the widget renders twice correctly, but it's also appending two <div id="google_translate_element2"> elements to the first widget on the page, see screenshot below:

After applying 🐛 Refactor to use libraries and compatibility with Big Pipe Needs work , it seems to do better in that the "google_translate_element2" element gets added after each corresponding widget, but it still has the problem that the same ID is being used twice. This is causing a ding by our accessibility testing tools, and it's malformed HTML to use the same ID twice.

ckrina → credited quicksketch → .

In Backdrop-land we ended up porting the code used by CKEditor's Source plugin.

That port exists in Backdrop here: ckeditor5.formatter.js.

When the content is saved to the field, we run the formatter, like this:

      let newData = editor.getData();
      newData = Backdrop.ckeditor5.formatHtml(newData);
      editor.destroy();
      element.value = newData;

I also considered/tried building the formatting into the HTML engine (we ported DrupalHtmlEngine into our module as well). But I ran into a different problem in that when using CKEditor's "Source" button, the internal formatter expects all the HTML to be on one line. If you run the formatter on already-formatted HTML, you end up with lots of issues like double-indented lines and extra new lines added when viewing the source within CKEditor.

quicksketch → created an issue.

Not sure if you remember, but one time I asked you for a "quick sketch" at sandcamp and you drew a portrait of me. I still have it <3

Hahaha! I do remember. I think you're the only person that's ever asked me for that. I recall being pretty disappointed with my output, I haven't been doing much drawing for a couple decades now. Ha!

I'll try out the latest PR again. Thanks!

Awesome, I think that alone will be a big help.

Thanks Tim!

quicksketch → created an issue. See original summary → .

I filed a PR that adds hover styling to the block sidebar and to the drop targets. The styling is very basic but it's a substantial UX improvement to give users visual indication both that a block is draggable and that a drop zone is droppable.

quicksketch → created an issue.

quicksketch → created an issue.

I read through and tried out the PR. The code rearrangement looks great; I think they'll be a lot of benefits to overriding classes like UpdateBlockForm and ConfigureSectionForm, allowing for other changes in the future. I also like the suppression of the layout changes message so there aren't duplicate messages. Seems like a lot of improvements are in the PR.

In testing, I encountered some issues:

• The settings page at admin/config/content/layout-builder-plus for some reason now is empty; with the message: No entities have layout builder enabled., which is not accurate.
• Enabling Layout Builder on a content type throws a fatal error: Drupal\Component\Plugin\Exception\ContextException: The entity context is not a valid context. in Drupal\layout_builder\Plugin\SectionStorage\SectionStorageBase->getContextDefinition() (line 150 of core/lib/Drupal/Core/Plugin/ContextAwarePluginTrait.php). . This can reproduced by visiting admin/structure/types/manage/page/display/default, enabling Layout Builder, then clicking "Manage Layout".

Since I couldn't get to the list of blocks, it was hard to actually test the new functionality. It seems like this would be done by adding a new "Layout Block"? This opens up some other questions/problems:

• How is this functionality enabled/disabled? Is this a new block that can put into the list of promoted blocks?
• Can nesting more than one level be prevented?
• Can only certain layouts be allowed to be nested? For example there's not much point in nesting a single column, and perhaps a site might make layouts that are made to be nested vs those that are made to be root-level

This is such a huge set of changes it might be valuable to separate out the storage and class refactoring into its own issue, then handle the UI problems of nested layouts on top of those changes.

quicksketch → created an issue.

It looks like for the most part, all attributes are already defined that editor_advanced_link needs, and they upcast and downcast fine. What's missing is that when setting or getting attributes for a link around an image, the attributes need to come from the image model, not the anchor. CKEditors LinkImage "absorbs" the wrapping <a> tag into the image model.

Although this is not a comprehensive solution, in _addExtraAttributeOnLinkCommandExecute() something like this needs to be executed when link command is fired:

        // If there is an image within this link, apply the link attributes to
        // the image model's htmlLinkAttributes attribute.
        const imageUtils = editor.plugins.get('ImageUtils');
        const closestImage = imageUtils.getClosestSelectedImageElement(selection);
        if (closestImage) {
          const htmlLinkAttributes = { attributes: extraAttributeValues };
          writer.setAttribute('htmlLinkAttributes', htmlLinkAttributes, closestImage);
        }

Note when setting htmlLinkAttributes the actual attribute names should be used (i.e. rel or id) rather than the model names (linkRel or linkId).

I ran into this same issue while porting link handling to Backdrop CMS (see https://github.com/backdrop-contrib/ckeditor5/issues/54). We started with the editor_advanced_link CKEditor plugin and adapted it for modal dialogs. Normal link handling works fine, but as is the case here, it does not work to apply attributes to images.

I'm investigating this in our own module, whatever solution we find I'll try to post back here.

In using this approach, I found that it is not perfect because $paragraph->getParentEntity() will return NULL on new nodes. getParentEntity() actually loads an entity from the database, and in the event that it's a new node, it's not in the database yet.

However, I found that my patch is still useful because it sets the parent entity type and field name, so I can set up a conditional like this:

function mymodule_form_layout_paragraphs_component_form_alter(&$form, FormStateInterface $form_state) {
    /* @var \Drupal\paragraphs\Entity\Paragraph $paragraph */
  $paragraph = $form['#paragraph'];
  $parent_field_name = $paragraph->get('parent_field_name')->value;
  if ($parent_field_name === 'field_page_content') {
    $form['field_image']['#access'] = FALSE;
  }
}

And it's possible that getParentEntity() may be refactored to store the actual entity instead of loading from the database if 🐛 Cannot access parent entity in hook_entity_create Needs review is completed.

Here's a patch version in addition to the MR.

The fix is pretty simple, we just need to set the paragraph parent entity right after creating the new paragraph but before displaying the edit form.

quicksketch → created an issue.

mcdruid → credited quicksketch → .

quicksketch → created an issue.

Select2 4.0.0 was released about 2 week ago. Looks like this issue back on. ;)

Sorry, yeah I kicked off the discussion in the wrong direction there. We can convert the textfield to a select *with JS*, then run Select2 on the new select list field we just created. However, that means we're going to have to adopt a lot of custom code to deal with converting a textfield to a select list. You'd need to account for single/multiple values, parsing the input field, converting it to a select list with the entered options selected. Then run Select2 on the resulting new select field and load the results via AJAX like we had been doing before. Before the values are submitted to the server, you have to convert these values *back* into a input so it would all come in as a single value in POST. A select list will send multiple values as an array.

Rather than attempt to revert the select back to input, we'd probably find it easier to maintain the original input on the page, and update it as the select list is updated.

So in summary, to use Select2:

- PHP outputs a textfield onto the page.
- Hide the original textfield and create a select list "copy" of the field with JS
- Apply Select2 to the select list, load results via AJAX
- On value change, update the original textfield.
- On POST, the server gets the value from the original field. The select element we could leave with no name attribute, so it would be nothing but a client-side widget for manipulating the hidden field.

So basically we'd re-implement the functionality that was removed from Select2 to support hidden fields.

I don't know... it's possible but doesn't seem right.

I just spent a day trying to upgrade the D7 Select2 module to support the new 4.0.0 version of the library. It *really* doesn't like working with input fields. Dropping support for (hidden) inputs is actually listed as a feature of the new version: https://select2.github.io/announcements-4.0.html#hidden-input

In trying to use it on input textfields, Select2 simply has no support whatsoever now. It actually tries to embed <option> tags inside of the textfield <input> element. The only hope we'd have is doing as Wim suggested and replace the textfields with select elements.

I don't know if this is going to be a suitable replacement for autocomplete text fields. A select list isn't going to be the right element when dealing with large lists, which is pretty much any situation we have autocomplete currently: author name, term tags, the search on api.drupal.org, etc.

The whole situation is disappointing. The "best" library for autocomplete basically doesn't want to deal with autocomplete and wants to focus exclusively on replacing select list elements. I suppose it shouldn't be too surprising, considering their past claims that autocomplete/type-ahead is outside the scope of their goals: https://github.com/select2/select2/issues/17.

This looks like a great direction.

Looking at the library and the patch, what approach should be used to bring consistency to non-Select2 select lists and Select2-enabled select lists? @aspilicious's image in #55 shows the differences pretty clearly. I'm guessing the default styling for Select2 would be left in place (considering there are no "skins") and the theme would take on the responsibility of styling them individually (as Seven theme styles select lists already).

Yeah the #type = managed_file element has mitigated this problem significantly, since most uploads would want to use the more advanced managed_file element anyway (who wouldn't want an AJAX-upload and progress bar?) I personally don't think this is a "major" issue any more and it's not affecting nearly as many sites now since File module is available in core to provide the majority use-case. This seems like just an inconvenience now.

Updated patch that has been tested.

Ha, this was "fixed" 3 years(!) ago in #64984-4: File Uploads fail when field is set to "required" → , but it never made it into core. Sad, sad. Here's a reroll of my old patch, not yet tested, but just putting this on the radar.