EZP-29998: As a developer, I want to define icons for content types #804

adamwojs · 2019-01-17T08:25:24Z

Question	Answer
Tickets	https://jira.ez.no/browse/EZP-29998
Bug fix?	no
New feature?	yes
BC breaks?	no
Tests pass?	yes
Doc needed?	yes
License	GPL-2.0

Example configuration

ezpublish:
    system:
       default:
           content_type:
              article:
                 thumbnail: '/assets/images/customarticle.svg'
              poll:
                 thumbnail: '/assets/images/poll.svg'

Using content type icons in Twig templates

Content type icons are accessible in Twig templates via ez_content_type_icon function which requires content type or content type identifier as an argument and returns path to content type icon.

<svg class="ez-icon ez-icon-{{ contentType.identifier }}">
    <use xlink:href="{{ ez_content_type_icon(contentType.identifier) }}"></use>
</svg>

If the icon for given content type wasn't specified in the configuration then path default icon will be returned.

Default content type icon

A configuration of the default icon for content types is possible via default-config key. For example:

ezpublish:
    system:
       default:
           content_type:
              default-config:
                 thumbnail: '/assets/images/mydefaulticon.svg'

Using content type icons in JS

Icons confgiguration is pushed from the backend as as part of the global configuration eZ.adminUiConfig.contentTypes.

However, more convenient way of retrieving icon would be to use new helper function getContentTypeIcon, which takes content type identifier as argument. It belongs to global object eZ.helpers.contentType.

Example with getContentTypeIcon:

const contentTypeIcon = eZ.helpers.contentType.getContentTypeIcon(contentTypeIdentifier);

return (
    <svg className="ez-icon">
        <use xlinkHref={contentTypeIcon} />
    </svg>
);

Checklist:

Coding standards ($ composer fix-cs)
Ready for Code Review

lserwatka · 2019-01-17T09:56:06Z

We might need to rethink naming for this function ezplatform_admin_ui_content_type_icon

I would suggest something like ez_render_icon, ez_ui_content_type_icon, or ez_content_type_icon

src/lib/UI/Service/ContentTypeIconResolver.php

…types

adamwojs · 2019-01-21T15:51:35Z

PR updated with the following changes:

Renamed twig helper function to ez_content_type_icon according to @lserwatka suggestion
ez_content_type_icon returns url decorated by assets component
Fixed configuration overwrite bug

dew326 · 2019-01-24T08:52:11Z

src/bundle/Resources/config/services.yml

@@ -21,6 +21,10 @@ imports:
    - { resource: services/permissions.yml }
    - { resource: services/forms.yml }

+parameters:
+    # default content type icon
+    ezsystems.ezplatform_admin_ui.default_content_type_thumbnail: '/bundles/ezplatformadminui/img/ez-icons.svg#file'


Why 'file' is default content type?

It was specified in the task (https://jira.ez.no/browse/EZP-29998) description:

In case of a custom content type without defined custom icons, then the file icon from admin-ui will be used.

dew326 · 2019-01-24T09:43:27Z

src/bundle/Resources/views/layout.html.twig

@@ -162,6 +162,7 @@
            '@EzPlatformAdminUiBundle/Resources/public/js/scripts/helpers/request.helper.js'
            '@EzPlatformAdminUiBundle/Resources/public/js/scripts/helpers/notification.helper.js'
            '@EzPlatformAdminUiBundle/Resources/public/js/scripts/helpers/timezone.helper.js'
+            '@EzPlatformAdminUiBundle/Resources/public/js/scripts/helpers/content.type.icon.helper.js'


I would name this just content.type.helper.js because in the addConfig() the group for those methods is helpers.contentType, which implies there will be different methods for content types not only of the icon.

I was thinking about the same and I agree.

ViniTou · 2019-01-24T09:43:54Z

src/lib/UI/Service/ContentTypeIconResolver.php

+    /**
+     * @param \eZ\Publish\Core\MVC\ConfigResolverInterface $configResolver
+     * @param \Symfony\Component\Asset\Packages $packages
+     * @param string|null $defaultThumbnail


doc does not match constructor parameter.

Similiar tihing seems to be in getContentTypeIcon method

Fixed in c10c434

dew326 · 2019-01-24T09:49:08Z

src/lib/UI/Service/ContentTypeIconResolver.php

+            $contentType = $contentType->identifier;
+        }
+
+        $thumbnail = null;


Here could be $thumbnail = $this->defaultThumbnail; and the if empty($thumbnail) is not needed.

Not really. If you configure null or empty string as the content type icon then default icon should be used.

dew326 · 2019-01-24T09:52:55Z

src/lib/UI/Service/ContentTypeIconResolver.php

+    public function getContentTypeIcon($contentType): ?string
+    {
+        if ($contentType instanceof ContentType) {
+            $contentType = $contentType->identifier;


This looks odd one time under the $contentType is ContentType object and once is the identifier. The ContentType seems to be not needed here so getContentTypeIcon() shouldn't take just string with an identifier?

Fixed in c10c434

adamwojs · 2019-01-25T12:45:26Z

PR updated according to @ViniTou and @dew326 suggestions.

dew326 · 2019-01-25T12:56:17Z

src/lib/UI/Service/ContentTypeIconResolver.php

+     *
+     * @return string|null
+     */
+    public function getContentTypeIcon(string $contentType): ?string


$contentTypeIdentifier or just $identifier?

mikadamczyk · 2019-01-28T10:35:18Z

As we talked, we can add the configuration for the default icon as part of the content_type configuration.

…types

adamwojs · 2019-01-29T10:24:26Z

As we talked, we can add the configuration for the default icon as part of the content_type configuration.

Done in 3b91172: default configuration is available under default-config key.

I also added unit tests for EzSystems\EzPlatformAdminUi\UI\Service\ContentTypeIconResolver.

mikadamczyk · 2019-01-29T11:09:26Z

src/lib/UI/Service/ContentTypeIconResolver.php

+{
+    private const DEFAULT_IDENTIFIER = 'default-config';
+    private const PARAM_NAME_FORMAT = 'content_type.%s';
+


do we need this empty line?

m-tyrala

QA approve

lserwatka · 2019-01-29T14:42:40Z

@DominikaK this should be documented.

…zsystems#804) * EZP-29998: As a developer, I want to define icons for content types * fixup! EZP-29998: As a developer, I want to define icons for content types * fixup! EZP-29998: As a developer, I want to define icons for content types * Add JS helper function returning content type icon href. * Fix jsDoc comment * Add default content_type icons for in-house Content Types * Fix content type icon helper * fixup! EZP-29998: As a developer, I want to define icons for content types * Applied CR suggestions * fixup! Applied CR suggestions * Fix JS - CR * fixup! EZP-29998: As a developer, I want to define icons for content types

adamwojs added the Feature label Jan 17, 2019

adamwojs assigned adamwojs and tischsoic Jan 17, 2019

tischsoic changed the title ~~EZP-29998: As a developer, I want to define icons for content types~~ [WIP] EZP-29998: As a developer, I want to define icons for content types Jan 17, 2019

tischsoic added the Work in progress label Jan 17, 2019

adamwojs commented Jan 17, 2019

View reviewed changes

src/lib/UI/Service/ContentTypeIconResolver.php Outdated Show resolved Hide resolved

adamwojs and others added 6 commits January 18, 2019 10:22

EZP-29998: As a developer, I want to define icons for content types

8a30940

fixup! EZP-29998: As a developer, I want to define icons for content …

13e7bb3

…types

fixup! EZP-29998: As a developer, I want to define icons for content …

5c0c9dd

…types

Add JS helper function returning content type icon href.

3aa6209

Fix jsDoc comment

6263aa5

Add default content_type icons for in-house Content Types

d3e978e

This was referenced Jan 21, 2019

EZP-29999: As an editor, I want to see custom icons for content types #805

Merged

EZP-29999: As an editor, I want to see custom icons for content types ezsystems/ezplatform-admin-ui-modules#132

Merged

tischsoic and others added 2 commits January 21, 2019 16:07

Fix content type icon helper

92be400

fixup! EZP-29998: As a developer, I want to define icons for content …

a26f9e8

…types

adamwojs changed the title ~~[WIP] EZP-29998: As a developer, I want to define icons for content types~~ EZP-29998: As a developer, I want to define icons for content types Jan 21, 2019

adamwojs added Ready for review and removed Work in progress labels Jan 21, 2019

adamwojs requested review from webhdx, ViniTou, dew326, mikadamczyk, Nattfarinn, pawbuj and sunpietro January 24, 2019 08:50

dew326 reviewed Jan 24, 2019

View reviewed changes

ViniTou reviewed Jan 24, 2019

View reviewed changes

dew326 reviewed Jan 24, 2019

View reviewed changes

Applied CR suggestions

c10c434

dew326 reviewed Jan 25, 2019

View reviewed changes

fixup! Applied CR suggestions

4ed6a8c

dew326 approved these changes Jan 25, 2019

View reviewed changes

lserwatka approved these changes Jan 25, 2019

View reviewed changes

Fix JS - CR

288f06e

mikadamczyk approved these changes Jan 28, 2019

View reviewed changes

fixup! EZP-29998: As a developer, I want to define icons for content …

bcab1f0

…types

mikadamczyk approved these changes Jan 29, 2019

View reviewed changes

adamwojs added Ready for QA and removed Ready for review labels Jan 29, 2019

m-tyrala approved these changes Jan 29, 2019

View reviewed changes

m-tyrala added QA approved and removed Ready for QA labels Jan 29, 2019

adamwojs added the Ready for MERGE label Jan 29, 2019

lserwatka merged commit eee23b4 into ezsystems:master Jan 29, 2019

MagdalenaZuba mentioned this pull request Jan 30, 2019

EZP-29998 As a developer, I want to define icons for Content Types ibexa/documentation-developer#520

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-29998: As a developer, I want to define icons for content types #804

EZP-29998: As a developer, I want to define icons for content types #804

adamwojs commented Jan 17, 2019 •

edited

Loading

lserwatka commented Jan 17, 2019 •

edited

Loading

adamwojs commented Jan 21, 2019

dew326 Jan 24, 2019

tischsoic Jan 24, 2019

dew326 Jan 24, 2019

tischsoic Jan 24, 2019

ViniTou Jan 24, 2019

adamwojs Jan 25, 2019

dew326 Jan 24, 2019

adamwojs Jan 25, 2019

dew326 Jan 24, 2019

adamwojs Jan 25, 2019

adamwojs commented Jan 25, 2019

dew326 Jan 25, 2019

mikadamczyk commented Jan 28, 2019

adamwojs commented Jan 29, 2019

mikadamczyk Jan 29, 2019

m-tyrala left a comment

lserwatka commented Jan 29, 2019

EZP-29998: As a developer, I want to define icons for content types #804

EZP-29998: As a developer, I want to define icons for content types #804

Conversation

adamwojs commented Jan 17, 2019 • edited Loading

Example configuration

Using content type icons in Twig templates

Default content type icon

Using content type icons in JS

Checklist:

lserwatka commented Jan 17, 2019 • edited Loading

adamwojs commented Jan 21, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamwojs commented Jan 25, 2019

Choose a reason for hiding this comment

mikadamczyk commented Jan 28, 2019

adamwojs commented Jan 29, 2019

Choose a reason for hiding this comment

m-tyrala left a comment

Choose a reason for hiding this comment

lserwatka commented Jan 29, 2019

adamwojs commented Jan 17, 2019 •

edited

Loading

lserwatka commented Jan 17, 2019 •

edited

Loading