Add a thumbnail/icon field to Paragraphs type

Added icon field for paragraph type. I can not set default icon because getIconFile() return null from unknown reason for me at the moment.
I need to fix that and add test coverage.

This is the implementation issue of the META about icons. Introducing icons is more than just add them to the types and then use them in the "add" widget. With other features upcoming, the META topic will grow and there are things that need discussion about the icon concept in general.

Fixed problem with functionality, only need to write test coverage.
Providing screenshots.

I already mentioned in another issue that I really like the idea to have icons or similar to be able to visually identify the paragraph type.

However I do not like the proposal to mix content and configuration, because we would have a dependency between a content entity (the file entity for the icon/image) and the configuration (entity) which is the bundle. That rather bad for development and integration.

Could we just use that pattern that we have for the site logo for example, where you can provide a path? That way the might have to make sure that the path is valid for all environments, but it is still configuration and not content..

This is done exactly like embed.module does it, and it works pretty well there. We're not the first to have content dependencies in config, views, blocks and other things have this too.

Providing default icons can be done with the default_content module for example.

1. +++ b/config/schema/paragraphs_type.schema.yml
   @@ -8,6 +8,9 @@ paragraphs.paragraphs_type.*:
   +    icon_uuid:
   +      type: string
   +      label: 'Icon uui'
    
   

   incomplete label

2. +++ b/src/Controller/ParagraphsTypeListBuilder.php
   @@ -25,6 +29,14 @@ class ParagraphsTypeListBuilder extends ConfigEntityListBuilder {
   +        '#alt' => $this->t('Icon for the @label button.', ['@label' => $entity->label()]),
   

   this is not a button.

3. +++ b/src/Entity/ParagraphsType.php
   @@ -54,4 +55,29 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +   * UUID of the button's icon file.
   

   nope

4. +++ b/src/Entity/ParagraphsType.php
   @@ -54,4 +55,29 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +  public $icon_uuid;
   

   lets make this protected

5. +++ b/src/Entity/ParagraphsType.php
   @@ -54,4 +55,29 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +  public function getIconUrl() {
   +    if ($image = $this->getIconFile()) {
   +      return file_create_url($image->getFileUri());
   +    }
   +  }
   

   we probably want to have a generic fallback token.

We also need a calculateDependencies() method.

Another note on #8/7. We will need to figure out how to support default icons, and while I can imagine we could add a path setting too, it won't really help without having support for module:// stream wrappers as we can't hardcode the path to files in the module, so we still need to copy them to the public folder and then we can just as well make them managed.

We should also register usages of those icons using file_usage service.

Usage: Yeah what i have seen is that usage was still temporary, so it would be deleted on cleanup cron.

Are we going to handle the default icon complexity in a follow-up?

Fine with doing that in a follow-up. We can already do something like have a file in the module, copy it to public:// and save as a file entity, then reference it. For the demo module, for example (which we can also do later, since we don't actually use the icons yet anyway)

Added test coverage and implement comm #9. Created follow -up from #11

Added calculateDependencies(). Now is ready for review.

Cleaning patch.

Move test in ParagraphsTypesTest and added dependency for file module.

1. +++ b/paragraphs.install
   @@ -180,4 +180,11 @@ function paragraphs_update_8008() {
   +/**
   + * Install file module.
   + */
   +function paragraphs_update_8010() {
   +  \Drupal::service('module_installer')->install('file');
   +}
   

   Doesn't look you tested this because that's now how install() works.

2. +++ b/src/Entity/ParagraphsType.php
   @@ -67,7 +67,7 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
      public function getIconFile() {
        if ($this->icon_uuid) {
   -      return $this->entityManager()->loadEntityByUuid('file', $this->icon_uuid);
   +      return \Drupal::service('entity.manager')->loadEntityByUuid('file', $this->icon_uuid);
        }
   

   you still use entity.manager.

Implemented comm #17.

+++ b/src/Tests/ParagraphsTypesTest.php
@@ -38,4 +38,31 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
+
+    // Check if icon is saved.
+    $this->drupalGet('admin/structure/paragraphs_type');
+    $this->clickLink('Edit');
+    $this->assertText('image-test-transparent-indexed.gif');
+  }

I'm not seeing any test coverage that the icon is displayed on the overview.

Added test case to check if image is presented in list builder. I am asserting if alt attribute is presented, so img element is also presented. I hope that this is enough.

I'm getting 2 info messages, is this correctly set?

+++ b/src/Tests/ParagraphsTypesTest.php
@@ -38,4 +38,34 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
+    // If alt attribute is presented image tag si also presented.

is*

+++ b/src/Tests/ParagraphsTypesTest.php
@@ -38,4 +38,34 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
+    $this->assertTrue((bool) $this->xpath("//img[@alt='Icon for the Test paragraph type paragraph type.']"), 'Alt text found');

For image assertion, try to get the path of the image and then assert it.
Maybe something like this example I found:

    $elements = $this->xpath('//img[@src="http://local/image.png"]');
    $this->assertEqual(count($elements), 1, 'Image plugin tip found.');

+++ b/src/Form/ParagraphsTypeForm.php
@@ -41,6 +41,21 @@ class ParagraphsTypeForm extends EntityForm {
+        'file_validate_extension' => ['gif png jpg jpeg'],
+        'file_validate_image_resolution' => ['32x32', '16x16'],

That's far from the expected use of the icon.

The recommended format should be SVG.
And the add widget will display those icons in a way bigger element than 32 pixels.
We might display it downscaled to a tiny size in buttons, but that's IMHO not relevant and should not go through an image style as SVG need to stay vectors IMHO. Not sure how to properly limit / define the requirement / validation in the UI.

Hint: Also Thunder wants to display the icons bigger in the preview / collapsed paragraph, see #2828110: Sorting of paragraphs 03 - create a NEW paragraph "in place" between existing ones

I'd say don't limit the resolution to start, we can always add code to automatically downsize like image fields support when we know how we use the icons and how big they're displayed?

I removed resolution validation and now is looking very bad, and also changed the file extension validation to be on 'svg' but still we can upload other types like png. jpg and so on.
Providing screenshot.

Yeah. That looks bad, but the image you uploaded as an example is also highly confusing. ;-)

I'm not sure about column sequence.

We will usually display Icon + Label. It might be best to start the row with an icon column prior to the label.
What also confuses me is the column "PARAGRAPHS TYPES". That should simply be label.

Hardcoded width and height for image to scale automatically, and also added css to decrease the width for column where is a icon.
Now is looking better. Also changed the order of column.
Providing screenshot.

Changed css class name for icon field.

From the image above i can see that the icon adds some padding below and that's why the padding / white room is not balanced.
Please check the vertical paddings / margins and adjust them.

Centering image.

Changed indentation in css.

After discussion with Sascha and Ivica we deal to do on this way.

Fixed the last patch, as it failed for me in src/Form/ParagraphsTypeForm.php, at line 73. Patch #33 has this:

if ($behavior_plugins = $this->paragraphsBehaviorManager->getDefinitions()) {

As opposed to this (8.x-1.x-dev):

if ($behavior_plugins = $this->paragraphsBehaviorManager->getApplicableDefinitions($paragraphs_type)) {

It was changed here: http://cgit.drupalcode.org/paragraphs/commit/?id=7aaa294ec7284daebe4eb3d...

I could not create an interdiff as #33 breaks, but I fixed this line, a typo and removed an extra space.

Can't stay at RTBC without interdiff or another review by someone.

@miro_dietiker
The only difference between patch #33 and #35 is this line:

if ($behavior_plugins = $this->paragraphsBehaviorManager->getDefinitions()) {

to this:
if ($behavior_plugins = $this->paragraphsBehaviorManager->getApplicableDefinitions($paragraphs_type)) {

Ran with $ diff add_a_thumbnail_icon-2830016-33.patch add_a_thumbnail_icon-2830016-35.patch command.

Now this needs a reroll as there is a conflict in ParagraphsTypeForm. Just tested if the patch works otherwise: not sure if I'm missing a step, but after enabling paragraphs_demo (on a clean env), applying the patch and then running drush updb, it gives me the following exception:

exception 'PDOException' with message 'SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal8.batch' doesn't exist'
...

Next exception 'Drupal\Core\Database\DatabaseExceptionWrapper' with message 'SQLSTATE[42S02]: Base table or view not found: 1146 Table 'drupal8.batch' doesn't exist: INSERT INTO {batch} (bid, timestamp, token, batch) VALUES
(:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2, :db_insert_placeholder_3);
...

Rebasing patch,
@gpap:
That is fixed in some previous patch.
@tduong:
For me works fine, I can't reproduce that error.

Rebasing again, experimental widget are committed.

No, the icon field will be there always and affect every Paragraphs user.
We will only use the field in the experimental widget, but that's a different issue.

@miro_dietiker, what do we need to do to get this in?

1. +++ b/src/Controller/ParagraphsTypeListBuilder.php
   @@ -14,8 +14,12 @@ class ParagraphsTypeListBuilder extends ConfigEntityListBuilder {
   -    $header['label'] = $this->t('Paragraphs types');
   

   Unrelated change, but I agree this should be done.

2. +++ b/src/Entity/ParagraphsType.php
   @@ -94,6 +121,20 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +  public function getDependencies() {
   +    parent::calculateDependencies();
   +
   +    // Add the file icon entity as dependency if an UUID was specified.
   +    if ($this->icon_uuid && $file_icon = \Drupal::service('entity.repository')->loadEntityByUuid('file', $this->icon_uuid)) {
   +      $this->addDependency($file_icon->getConfigDependencyKey(), $file_icon->getConfigDependencyName());
   +    }
   +
   +    return $this->dependencies;
   +  }
   

   This is weird, the getDependencies method is now responsible for adding dependencies. Should this not be calculateDependencies?

3. +++ b/src/ParagraphsTypeInterface.php
   @@ -36,4 +36,28 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +  /**
   +   * Returns the button's icon file.
   +   *
   +   * @return \Drupal\file\FileInterface
   +   *   The file entity of the button icon.
   +   */
   +  public function getIconFile();
   +
   +  /**
   +   * Returns the URL of the button's icon.
   +   *
   +   * If no icon file is associated with this Embed Button entity, the embed type
   +   * plugin's default icon is used.
   +   *
   +   * @return string
   +   *   The URL of the button icon.
   +   */
   +  public function getIconUrl();
   +
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public function calculateDependencies();
   +
   

   Do we really need to add these to ParagraphsTypeInterface and ParagraphsType?

   calculateDependencies is already in ConfigEntityInterface. getIconFile and getIconUrl contain very little code, having extra methods to return these oneliners seems kind of pointless?

+++ b/src/ParagraphsTypeInterface.php
@@ -36,4 +36,28 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
+  public function calculateDependencies();

This is already part of ConfigEntityInterface.

Now that the release is out, this should be the next step to unlock use of icons.

1. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -77,6 +77,20 @@ class ParagraphsTypeForm extends EntityForm {
   +        'file_validate_extension' => ['svg'],
   

   This should be file_validate_extensions

2. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
   @@ -38,4 +38,34 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
   +  /**
   +   * Test if paragraph type icon.
   +   */
   +  public function testParagraphTypeIcon() {
   

   This doesn't apply? Not sure why but we should check that.

Besides that there is an issue with the configuration. The uuid for the file is exported, but the file it is actually linked to isn't. This way you get a broken icon after a release.

This makes me think about the whole approach of using file uploads for this. I guess this is done so you don't have to write code to get a icon? But now you need to know where to find icons, and somehow match them to the admin theme you are using. Even if we find a way to deploy icons, uploading icons for each paragraph type is not a very friendly way to do this.

We've had a short discussion here at the paragraphs sprint and came to the following conclusions:

• Uploading icons for each paragraph type is not very user friendly, choosing from a set of icons is a big improvement.
• Creating a library of icons is the next issue. We can try to provide as much icons as we can through the module (starting with a couple and expand the icon set in followups).
• You should be able to add/alter the base list of icons. A separate module can maybe provide a way to upload new icons, but paragraphs only allows add/alter through code (so uploading icons is out of scope for now).
• Themes should be able to override the icons.

I guess we should now discuss what the best way is to move forward.

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

Copy/paste 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.

Thank you for the proposal and discussing this at the NL sprint.

The proposal has a key problem: It adds huge complexity to handle a managed overridable set of icons. That dimension of complexity is a bad sign for an approach. If this doesn't exist yet, it should be introduced by a dedicated module (there is also project icon!) and otherwise we should focus on a minimum approach.

Using a file field is IMHO a much better start to iterate. It is simply available and users could start to upload icons. Then we could deal in follow-ups with...
- Providing a default action by code as a fallback for module shipped paragraph type
- Alter the widget to a predefined selection list (consider icon providers?)
- Support exporting references so that the icon is transported in export / import

I believe all these problems can be solved on top of the "icon as content" approach we proposed.
I'll pass the ball to Berdir to provide feedback too.

Yes, this solution isn't perfect, but it's based on the embed.module and I agree that it is a useful starting point.

We can still add a separate field for a filename or selecting from different provided default icons later on. That's exactly why those getIconUrl() and so on methods are important and must be kept, because they can then deal with those additional options without making the public API more complicated.

Deploying content is a problem, but this one case of many and it needs a better general solution. Fabian Bircher actually mentioned that he is working on a module in his session at the mountain camp (https://drupalmountaincamp.ch/sessions/configuration-management-without-...) and that he will release something soon.

Switch to use calculateDependencies() instead of getDependencies(), that was my fault. Added test for calculateDependencies(). I agree with @Berdir that this is enough for starting point. Other stuff we can do in follow-ups.

I think we are almost there. It would be nice to link the follow-ups to remove the file field and replace it with some better way of selecting icons. We can at least continue the discussion about it in the other issue.

At least the SVG validation should be fixed.

1. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -79,6 +79,20 @@ class ParagraphsTypeForm extends EntityForm {
   +        'file_validate_extension' => ['svg'],
   

   I think this should be file_validate_extensions.

2. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   * Returns the button's icon file.
   ...
   +   * Returns the URL of the button's icon.
   

   Nit: Is this just for buttons? Maybe we just remove the word button here.

3. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   *   True or False dependant on plugin state.
   

   Nit: TRUE if the plugin is enabled, FALSE otherwise.

BTW i had the idea of a paragraph type "separator" in the collection that has a related problem:
#2854358: [META] Fundamental Type: Separator
Similar to icons, we could offer a selection of vector separators that a user could choose. Thus, we would need a similar provider / selectioin.
One difference though: Icons should primarily be chosen due to their meaning / semantics. The separator is likely being selected only because of its appearance.

Small changes from #56 and created follow-up #2854585: Improve icon selection in paragraphs type form.

Forgot to set on needs review.

Fixing test failing.
After discussing with @Berdir added jpg and png as file extensions.

+++ b/src/ParagraphsTypeInterface.php
@@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
+   * Returns the URL of the's icon.

This is a small typo, but I think this can be fixed on commit. Nice work, RTBC!

I will create a follow up to discuss any improvements as suggested in #50.

I already created one issue for discussing. Here it is : #2854585: Improve icon selection in paragraphs type form.

Small things, plz fix.

1. +++ b/src/Controller/ParagraphsTypeListBuilder.php
   @@ -14,8 +14,12 @@ class ParagraphsTypeListBuilder extends ConfigEntityListBuilder {
   +    $header['icon_uuid'] = [
   
   @@ -23,6 +27,17 @@ class ParagraphsTypeListBuilder extends ConfigEntityListBuilder {
   +    $row['icon_file'] = '';
   

   The keys don't match, but they should.

2. +++ b/src/Controller/ParagraphsTypeListBuilder.php
   @@ -23,6 +27,17 @@ class ParagraphsTypeListBuilder extends ConfigEntityListBuilder {
   +        '#width' => 32,
   +        '#height' => 32,
   

   Does this distort the image if it is not square?

3. +++ b/src/Controller/ParagraphsTypeListBuilder.php
   @@ -23,6 +27,17 @@ class ParagraphsTypeListBuilder extends ConfigEntityListBuilder {
   +        '#alt' => $this->t('Icon for the @label paragraph type.', ['@label' => $entity->label()]),
   

   Too repetitive on table. The label is output separately. So for best accessibility this can even be an empty "".
   https://www.w3.org/WAI/tutorials/images/decorative/

2. Yes.

Fixed stuff from previous comment and convert css into sass.

Fixing test failing.

You are right.

Ops, that was wrong patch.

Here some feedback :P sorry for being too pedantic...

1. +++ b/src/Controller/ParagraphsTypeListBuilder.php
   @@ -14,8 +14,12 @@ class ParagraphsTypeListBuilder extends ConfigEntityListBuilder {
   +    $header['icon_file'] = [
   +      'data' => $this->t('Icon'),
   +    ];
   

   This doesn't break the 80 chars length rule, so you can do this in one line.

2. +++ b/src/Entity/ParagraphsType.php
   @@ -92,6 +119,20 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +    // Add the file icon entity as dependency if an UUID was specified.
   

   "... if a ..." not "an".

   and "... UUID is specified."

3. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -79,6 +79,20 @@ class ParagraphsTypeForm extends EntityForm {
   +      '#description' => $this->t('Icon for he paragraph type.'),
   

   Typo, but I would write "Paragraph type's icon." or even remove it since it doesn't say anything more useful than its title.

4. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -130,6 +144,18 @@ class ParagraphsTypeForm extends EntityForm {
   +    // If a file was uploaded to be used as the icon, get its UUID to be stored
   +    // in the config entity.
   

   Better something like "Set the file UUID to the paragraph configuration."

5. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -130,6 +144,18 @@ class ParagraphsTypeForm extends EntityForm {
   +    $status = $paragraphs_type->save();
   

   Just save it, no need to assign it to any variable.

6. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   * Returns the icon file.
   

   ".. entity."

7. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   *   The file entity of the button icon.
   

   "The icon's file entity."

8. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   * Returns the URL of the's icon.
   

   "Returns the icon's URL."

9. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   * If no icon file is associated with this Embed Button entity, the embed type
   +   * plugin's default icon is used.
   

   Copy/paste from EmbedButtonInterface.. Remove these lines since you don't provide any else-case.

10. +++ b/src/ParagraphsTypeInterface.php
    @@ -37,13 +37,32 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
    +   *   The URL of the button icon.
    

    "The icon's URL."

11. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -39,4 +41,39 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +   * Test if paragraph type icon.
    

    "Tests the paragraph type icon settings."

12. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -39,4 +41,39 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +    $admin_user = $this->drupalCreateUser(['administer paragraphs types']);
    +    $this->drupalLogin($admin_user);
    

    I wouldn't create a variable just to use it once.

13. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -39,4 +41,39 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +    $this->drupalGet('admin/structure/paragraphs_type');
    +    $this->clickLink(t('Add paragraphs type'));
    

    If you are not going to check anything between the paragraph types overview and the add form, then just do drupalGet('admin/structure/paragraphs_type/add').

14. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -39,4 +41,39 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +    // Check if icon is saved.
    

    "Check if the icon has been saved."

15. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -39,4 +41,39 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +    // If alt attribute is presented image tag is also presented.
    

    "The alt attribute and image tag should be present."

    But you should make it clearer in your assert that you are checking both alt and image tag values.

16. +++ b/src/Tests/Classic/ParagraphsTypesTest.php
    @@ -39,4 +41,39 @@ class ParagraphsTypesTest extends ParagraphsTestBase {
    +    $this->drupalGet('admin/structure/paragraphs_type');
    

    Not necessary. You are already on that page.

@tduong:

1. This is optional, sorry but I prefer like this.
2. This is debatable.
3. Typo?

2. it is "a UUID" and not "an UUID", see here the explanation/example ;)
3.

+++ b/src/Form/ParagraphsTypeForm.php
@@ -82,7 +82,7 @@ class ParagraphsTypeForm extends EntityForm {
-      '#description' => $this->t('Icon for he paragraph type.'),
+      '#description' => $this->t('Icon for the Paragraphs type.'),

The typo that you've fixed, but again, this description doesn't say anything more than the title. There is even an issue to #2849207: Drop redundant descriptions.

12. and 13. are not addressed yet?

12. IMHO it is more readable like this and if I put in one line, there will be more than 80 characters.

1. +++ b/src/Entity/ParagraphsType.php
   @@ -75,6 +83,16 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +  public function getIconFile() {
   +    if ($this->icon_uuid) {
   +      return \Drupal::service('entity.repository')
   +        ->loadEntityByUuid('file', $this->icon_uuid);
   

   We should have $this->entityTypeManager() here, should we use ...->getStorage('file')->loadByProperties() instead?

2. +++ b/src/Entity/ParagraphsType.php
   @@ -92,6 +119,20 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +    if ($this->icon_uuid && $file_icon = \Drupal::service('entity.repository')->loadEntityByUuid('file', $this->icon_uuid)) {
   

   Same as above.

Also, do we want to add some of those icons to demo paragraph types? Maybe can be done in follow-up

I hope this is what you mentioned.
Yes, would be nice if we add icons to paragraphs type in demo content, there is an issue #2860212: Add icons to paragraphs type in demo content.

Correct interdiff.

+++ b/src/Entity/ParagraphsType.php
@@ -85,8 +85,16 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
+      $icon_file = [];
+      if (count($files)) {

No need to count, empty arrays validate false. Also IMHO we should return FALSE here if we don't find a file. And the inteface docblock should be updated accordingly.

Yes, you are right.

A few things:

1. +++ b/src/Entity/ParagraphsType.php
   @@ -75,6 +83,23 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +        return $icon_file = array_shift($files);
   

   Don't assign to any variable if you just want to return it.

2. +++ b/src/Entity/ParagraphsType.php
   @@ -85,6 +110,15 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
   +    if ($image = $this->getIconFile()) {
   +      return file_create_url($image->getFileUri());
   +    }
   

   If getIconFile() else-case returns FALSE, shouldn't this method return FALSE as well?

3. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,29 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   *   The URL of the button icon.
   

   "The icon's URL, FALSE if no icon file is found."

EDIT: and needs a reroll

Made changes from #81.

Just a small thing:

+++ b/src/Entity/ParagraphsType.php
@@ -75,6 +83,23 @@ class ParagraphsType extends ConfigEntityBundleBase implements ParagraphsTypeInt
+      if ($files) {
+        return array_shift($files);
+      }
+    }
+
+    return FALSE;

Rethinking about this, I would just create $files = []; and do a single return line like return $files ? array_shift($files) : FALSE;

1. +++ b/src/Form/ParagraphsTypeForm.php
   @@ -130,6 +143,17 @@ class ParagraphsTypeForm extends EntityForm {
   +    $paragraphs_type->save();
   

   Why are we setting/saving stuff in validate()? Is there a reason we don't do this in save()?

2. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,29 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   * @return \Drupal\file\FileInterface
   

   If we also return FALSE, this should be reflected here.

3. +++ b/src/ParagraphsTypeInterface.php
   @@ -37,13 +37,29 @@ interface ParagraphsTypeInterface extends ConfigEntityInterface {
   +   * @return string
   +   *   The icon's URL or FALSE if icon does not exits.
   

   Same here

Also we could open a follow up to discuss how we should output the icon as mentioned in one of the comments. We just output the full size now.

Implemented comm #84 and created follow-up #2863592: Change how we output the paragraph type icon

Committed, thanks to everyone!
@toncic Could you open a follow up issue in paragraphs collection for that too?

Added some follow-ups.