Improve icon selection in paragraphs type form.

Copy paste of the comment in #2830016: Add a thumbnail/icon field to Paragraphs type.
-----------------
After discussion at the paragraphs sprint with jurcello, sutharsan and marcvangend we came to the following proposal.

What we need at a minimum:

• name: For storage purposes

How can we do it:

• We create a icon service to provide available icons.
• We discover icons in modules / themes in YAML files.
• Each module / theme can provide a [module/theme name].paragraphs.icons.yml which will be picked up by the icon service.
• Each icon contains settings like path and a weight, so we can override icons in other modules / themes.
• We provide an event to alter the outcome of the discovery.
• We add a configuration settings for a icon base dir (can be set from the interface like media entity) and copy all theme icons to the base dir on discovery + the icon with the heighest weight (can be module or theme).
• We change icon names with the module/theme name in the icon base dir like [icon name].[theme].[extension]. The heighest weight is just called [name].[extension].
• We store the result of the discovery, including the updated filename in cache.
• Each icon can contain an array of settings, by default most can define a path and/or weight, but also things like attributes or other settings can be provided.
• We provide a theme function that can use the settings of icons to render a icon. In theory a valid icon image is not needed when themes provide icons through a font or something like that.
• Eacht paragraph type can select their own paragraph icon (we store the name only), and you can reuse generic ones if needed.
• The active theme icon is always prefered. We fall back to the heighest weight if the theme doesn't provide a icon.

Example of [module/theme name].paragraphs.icons.yml:

text:
  path: '/relative/module/path.svg'
  weight: 0
image:
  path: '/assets/icons/image.svg'
  weight: 1
  attributes:
    id: 'example_id'
    class:
      - 'whatever'
      - 'class'
      - 'you'
      - 'want'
  some_property: some_value

And extra info from slack:
-----------

sutharsan
@seanb I do not understand “… + the icon with the heighest weight (can be module or theme).” in "We add a configuration settings for a icon base dir (can be set from the interface like media entity) and copy all theme icons to the base dir on discovery + the icon with the heighest weight (can be module or theme).”

@seanb About "We change icon names with the module/theme name in the icon base dir like [icon name].[theme].[extension]. The heighest weight is just called [name].[extension].”
I think that you should explain that we want themes to be able to override base theme(s) and modules. But also allow fallback if the (final) theme does not provide an icon.

seanb
@sutharsan I can see how this might be confusing. Since multiple modules/themes can implement icons with a specific name, the proposal is to only care about the current theme, or the icon with the highest weight as a fallback. Hopefully copy/pasting this to the issue is a good explanation.

As far as we could see, module/theme icons with lower weight/importance will not be used. Copying/returning/caching those seem pointless. We could off course be missing some edge cases here, so please let me know if there are examples where this could lead to undesired results.

The title and the text sounds really strange.

We have introduced APIs and want to keep them compatible. Also storage.

What we want to do is offer a better widget, icon selection, and better import / export compatibilits and default fallbacks.
But we want to avoid introducing complex icon specific generic management tools. Our approach should be very simple and we should figure out if there are tools for icon management we can reuse.
An example to consider is https://www.drupal.org/project/icon but i didn't yet check in deep detail.

We have introduced APIs and want to keep them compatible. Also storage.

#2830016: Add a thumbnail/icon field to Paragraphs type is not in yet? I think using icon uploads for each type (and using them for all themes) is very bad UX. If we can't remove the file field (because of bc/storage), I think we shouldn't add this in the first place.

Never mind, we can leave the file field and just add new fields with a better UX for icon selection. Updated the issue title/summary.

Here is a new approach: instead of struggling with providing a default icon using the configuration we could simply set the icon using a hook.

The attached patch introduces the hook hook_paragraphs_type_static_icon_uri_alter() that allows modules and themes to set an image for a ParagraphsType.
If someone uploads an image to the ParagraphsType the uploaded file is used. Otherwise the image set with the new hook will be displayed.

The hook approach from @stBorchert would be quite a step forward. Currently it's really terrible. You can only upload icons. You can't even set a URL or sth. That would also simplify a lot. The solution outlined in the summary is of course way more sophisticated and more "Drupal-ish". It reminds me of the breakpoint configuration which is also done by auto-discovering yml files... But I think it's probably quite out of reach due to its complexity, right? So perhaps the hook approach is *good enough(TM)*?

1. +++ b/paragraphs.api.php
   @@ -47,6 +47,20 @@ function hook_paragraphs_behavior_info_alter(&$paragraphs_behavior) {
   + */
   +function hook_paragraphs_type_static_icon_uri_alter(&$uri, ParagraphsTypeInterface $paragraphs_type) {
   +  if ('gallery' === $paragraphs_type->id()) {
   +    $uri = 'module://my_image_library/images/library/gallery.png';
   +  }
   

   module:// doesn't exist without core patches or other modules, so we shouldn't rely on that in our documentation.

2. +++ b/src/Entity/ParagraphsType.php
   @@ -117,10 +117,39 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
        }
   +    elseif ($uri = $this->getStaticIconUri()) {
   +      return $this->getStaticIconUrl();
   +    }
    
        return FALSE;
   

   this calls the method and with that the alter hook twice.

3. +++ b/src/Entity/ParagraphsType.php
   @@ -117,10 +117,39 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +   * {@inheritdoc}
   +   */
   +  public function getStaticIconUrl() {
   +    $uri = $this->getStaticIconUri();
   +
   +    /** @var Drupal\Core\StreamWrapper\StreamWrapperInterface $stream_wrapper */
   +    if ($uri && ($stream_wrapper = \Drupal::service('stream_wrapper_manager')->getViaUri($uri))) {
   +      return $stream_wrapper->getExternalUrl();
   +    }
   +
   +    return FALSE;
   +  }
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function getStaticIconUri() {
   +    $uri = NULL;
   +    \Drupal::moduleHandler()->alter('paragraphs_type_static_icon_uri', $uri, $this);
   +    if ($uri) {
   +      return $uri;
   +    }
   +    return FALSE;
   +  }
   

   no reason for two separate methods, just inline it.

   Also doesn't need to be a public API, the only public API is the one above.

   Also why doesn't this use file_create_url()?

4. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -100,8 +100,32 @@ class ParagraphsTypeForm extends EntityForm {
   +    $file = $paragraphs_type->getIconFile();
    
   -    if ($file = $this->entity->getIconFile()) {
   +    /** @var \Drupal\Core\Url $static_icon_url */
   +    if (!$file && $static_icon_url = $paragraphs_type->getStaticIconUrl()) {
   +      $form['static_icon_file'] = [
   

   ah, that's why you made the method public.

   Why not just call getIconUrl() and if it returns something, assume it's provided by module/theme?

   Note that the current alter hook doesn't actually allow themes, that needs to be explicitly if we want that.

5. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -100,8 +100,32 @@ class ParagraphsTypeForm extends EntityForm {
   +      ];
   +      $form['static_icon_file']['preview'] = [
   +        '#type' => 'html_tag',
   +        '#tag' => 'img',
   +        '#attributes' => [
   +          'class' => ['paragraphs-static-icon'],
   +          'src' => $static_icon_url,
   +          'width' => '100px',
   +        ],
   

   why not use #theme => 'image'?

Updated the patch based on Saschas notes.

Would it be possible to explore using the Media Library for this?

It seems that replacing this with something like a media field would solve most of the problems people have while providing a really easy to use interface. It would even allow for the creation of icon packs that could be packaged in a module and loaded on a site.

Of course this would mean a relatively trivial feature of Paragraphs would introduce a dependency on the Media Library.

Using the media library, the icons would be content instead of config and that's something we want to avoid since that makes it a lot harder to deploy to different environments. I'm a fan of the current hook-based approach. I have been using it on a production site for a while now, could we consider merging this? It's a simple solution that allows for a lot more flexibility than the current one.

Overall this is great. But I see a few tweaks that need to be made.

    \Drupal::moduleHandler()->alter('paragraphs_type_static_icon_uri', $uri, $this);

This only allows modules to alter. But icons are often more of a theme responisibility. I checked and there's no \Drupal::themeHandler(), but maybe it's a service???

    $this->t('Static @icon provided by module or theme.', ['@icon' => $this->t('Paragraph type icon')])

This placeholder "Paragraph type icon" assumes that all languages will structure those words together. I think the Drupal standard is to just have it as one big string without the placeholder. "Static Paragraph type icon...". And since the same string is reused a few lines down, best to assign it to a variable and reuse.

@dalin I processed your feedback. I also replaced the usage of file_create_url, assuming this won't be merged before #3292632: Drupal 10 compatibility.

Tried to apply the patch on Drupal 10.1.x-dev
Patch failed to apply

error: while searching for:

    // Allow other modules to alter the icon URI.
    \Drupal::moduleHandler()->alter('paragraphs_type_static_icon_uri', $uri, $this);

    return $uri;
  }

error: patch failed: src/Entity/ParagraphsType.php:201
error: src/Entity/ParagraphsType.php: patch does not apply
Checking patch src/Form/ParagraphsTypeForm.php...
Checking patch src/Entity/ParagraphsType.php...
error: while searching for:
      return file_create_url($image->getFileUri());
    }
    elseif ($uri = $this->getStaticIconUri()) {
      return file_create_url($uri);
    }

    return FALSE;

error: patch failed: src/Entity/ParagraphsType.php:187
error: src/Entity/ParagraphsType.php: patch does not apply

Re-rolled patch against latest 8.x-1.x-dev branch.

The patch from #17 misses a lot of code changes from #10, therefore hiding it. The MR diff contains all the code changes from #10 and also fixes the deprecated usage of file_create_url(). You should use the MR and its diff instead.

I rebased branch 2854585-improve-icon-selection with 8.x-1.x and improved the calls to file_create_url() resp. the file_url_generator service.

Just found a small bug with this patch: The following element is added to ParagraphsTypeForm::form() method's form array by this patch:

<?php
$form['static_icon_file']['description'] = [
  '#type' => 'item',
  '#description' => $description,
];

Unfortunately an element with '#type' => 'item' is treated as an input value and thus interferes with the form's actual description form field value.

So my changes add '#input' => FALSE to the new $form['static_icon_file']['description'] element to NOT treat this element as a form value anymore.

I'd really like a way to set icons that don't have to be uploaded and stored in the files directory. It would be ideal to be able to just specify a path to an icon, so it could be included with the theme. I would also like the option of using something like fontawesome to provide icons, so being able to just put in an HTML tag that displays an icon would be great. Then it's on me to make sure that the icon font is included with the admin theme for the sites I build, which I already do and use in other areas.

@teknocat Hi. Did you had a change to test the merge request we've made?
After applying the changes you are able to implement hook_paragraphs_type_static_icon_uri_alter() (in a module or theme) where you can set the icon path to wherever you want. The file could be saved in a module, in the a theme, on an external storage, you name it ...

To use icons provided by fontawesome, you'll nee to override/decorate the ParagraphsTypeListBuilder class and change the way, $row['icon_file']['data'] is build.
Example (not tested):

<?php
      $row['icon_file']['data'] = [
        '#theme' => 'html_tag',
        '#tag' => 'span',
        '#attributes' => [
          'class' => ['fa-' . $entity->id()],
        ]
      ];
?>

This will create a <span> tag using the paragraph type ID as fontawesome class.

@stborchert ah ok! Sorry I don't think I fully understood the possibilities with that merge request.

I'll try that out.

Hello together 👋 What do you think about changing the direction here completely and make use of Drupal's new API for icon management?

That would allow themes or modules to define icon packs that can be easily referenced in the Paragraph type configuration like my_icon_pack_name:gallery and can be rendered via PHP or Twig:

PHP:

$build['icon'] = [
   '#type' => 'icon',
   '#pack_id' => 'my_icon_pack_name',
   '#icon_id' => 'gallery',
   '#settings' => [
     'size' => 64,
   ],
 ];

Twig:

{{ icon('my_icon_pack_name', 'gallery', {size: 64}) }}

There is a really cool widget that is available when installing the UI Icons module that allows you to choose icons via autocomplete:

When storing simple IDs of icons we don't need to care about storing paragraph icons anymore and they can easily survive deployments 😏

@yannickoo

Sounds interesting.

Let's say that I'm creating a new paragraph bundle. Where do I put my icon? In the front-end theme? In the admin theme (That would be a hassle because then I'd need to sub-theme Gin)?

Also, I never actually use "icons" here, but rather screenshots. IMO that's a better user experience. How would we handle that case?

@dalin the cool thing about the new Icon API is that any extension can provide an icon pack so it might be totally enough to have a custom module with an images folder that contains screenshot of your different paragraph types.

You can easily let Drupal scan your folder for any *.png files and tell Drupal how to render your "icons" e.g.

paragraph_icons:
  extractor: path
  config:
    # One icon per listed source, allowed extensions: svg, png, gif, relative to
    # this file or to drupal root if start with a `/`. Remote icons are allowed too
    # Format can be mixed but only one template is possible. If same id (name here)
    # later will override.
    # If an icon does not exist, it will be ignored and a log message will be created.
    sources:
      - images/gallery.png
      - images/another-image.png
      - images/text-paragraph.gif
      - https://www.drupal.org/files/drupal-wordmark.png
  template: '<img src="{{ source }}" width="270" height="160" alt="{{ icon_id }}">'

That would register and provide four different screenshots that could be referenced like paragraph_icons:gallery, paragraph_icons:another-image, paragraph_icons:text-paragraph, paragraph_icons:drupal-wordmark.

No matter if we have nicely prepared SVG files or just screenshots, the new Icon API would allow us to expose them easily and use them by just their ID 🔥

I've rebased the MR and made some small adjustments.

Thanks stborchert! What would you recommend to support using Icon API here? Do you think there is a new module needed for the integration which is using paragraphs_type_static_icon_uri? Or should we adjust the current implementation with ModuleHandler::moduleExists('ui_icons')?