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

tischsoic · 2019-01-17T14:56:59Z

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

Requires: #804

UDW part in: ezsystems/ezplatform-admin-ui-modules#132
date-based-publisher: https://github.com/ezsystems/date-based-publisher/pull/89
Workflow: https://github.com/ezsystems/ezplatform-workflow/pull/46

Views with icons

Content item view & edit view

Bookmarks

Trash

Drafts

Content type list; content type view; content type edit

Dashboard

Checklist:

pass content types data to twig templates, which lack them
add ez_content_type_icon where necessary
ready for Code Review

sunpietro · 2019-01-24T12:20:42Z

src/bundle/DependencyInjection/Configuration/Parser/ContentType.php

+ * Example configuration:
+ *
+ * ```yaml
+ * ezpublish:


Can we change it to ezplatform now?

sunpietro · 2019-01-24T12:23:40Z

src/bundle/Resources/public/js/scripts/helpers/content.type.icon.helper.js

+        }, {});
+
+    /**
+     * Returns href to content type icon


Returns an URL to a content type icon

sunpietro · 2019-01-24T12:23:54Z

src/bundle/Resources/public/js/scripts/helpers/content.type.icon.helper.js

+     * @param {String} contentTypeIdentifier
+     * @returns {String|null} href to icon
+     */
+    const getContentTypeIcon = (contentTypeIdentifier) => {


getContentTypeIconUrl

sunpietro · 2019-01-24T12:24:47Z

src/bundle/Resources/public/js/scripts/helpers/content.type.icon.helper.js

+            contentTypesDataMap = createContentTypeDataMap();
+        }
+
+        const iconHref = contentTypesDataMap[contentTypeIdentifier].thumbnail;


sunpietro · 2019-01-24T12:25:30Z

src/bundle/Resources/public/scss/_extra-actions.scss

@@ -160,7 +160,7 @@
            &-label {
                margin: 0;
                display: block;
-                padding-left: 1.5rem;
+                padding-left: 2.5rem;


Convert to pixels and use calculateRem function

sunpietro · 2019-01-24T12:25:39Z

src/bundle/Resources/public/scss/_instant-filter.scss

@@ -8,17 +8,29 @@
 .ez-extra-actions--create {
    .ez-instant-filter {
        &__group-name {
-            padding-left: .5rem;
+            padding-left: 0.5rem;


Same as above

sunpietro · 2019-01-24T12:28:14Z

src/bundle/Templating/Twig/ContentTypeIconExtension.php

+    /**
+     * {@inheritdoc}
+     */
+    public function getFunctions(): array


function getFunctions confuses me. What does it do?

This PR contains code from: #804 as it depends on it.

Please comment PHP parts in #804.
Corresponding line: https://github.com/ezsystems/ezplatform-admin-ui/pull/804/files#diff-a7ba97e75601073fc38f8daa145f316dR31

BTW It's Adam's part - ping @adamwojs.

But it seems to be inherited from Twig_Extension and it "Returns a list of functions to add to the existing list." (https://twig.symfony.com/doc/2.x/advanced.html)

In this way you are defining custom Twig functions provided by extension. As you me notice this class implement interface from the 3rd party library so "naming" is not subject to discussion.

tischsoic · 2019-01-30T12:19:18Z

Branch was rebased onto master and is ready for review.

dew326 · 2019-01-30T12:30:10Z

src/bundle/Resources/public/scss/_instant-filter.scss

+                top: 0;
+                left: calculateRem(20px);
+                width: calculateRem(16px);
+                height: calculateRem(16px);


You should use ez-icon--small

I defined it here in order to overwrite selector: .ez-context-menu .ez-sticky-container .ez-icon.

dew326 · 2019-01-30T12:37:00Z

src/bundle/Resources/views/admin/content_type/edit.html.twig

@@ -17,7 +17,7 @@
 {% block page_title_admin %}
    {% include '@ezdesign/parts/page_title.html.twig' with {
        title: 'content_type.view.edit.title'|trans({ '%name%': content_type.name })|desc('Editing Content Type: %name%'),
-        iconName: 'content-type'
+        contentType: content_type.identifier


It's not contentType. Variables in twig are with _.

There was already iconName in/views/parts/page_title.html.twig, this is why I opted for contentType to make it consistent within template file.

In this case consistent is not a good way.

OK, changed. :)

dew326 · 2019-01-30T12:41:23Z

src/bundle/Resources/views/parts/page_title.html.twig

@@ -5,6 +5,10 @@
            <svg class="ez-icon ez-icon-{{ iconName }}">
                <use xlink:href="{{ asset('bundles/ezplatformadminui/img/ez-icons.svg') }}#{{ iconName }}"></use>
            </svg>
+        {% elseif contentType is defined %}
+            <svg class="ez-icon ez-icon-{{ contentType }}">


Something is wrong here. From a line below contentType is an object not string. In the other template you pass identifier as a contentType.

You was right, very good catch, I'm impressed! 😄
Fixed in: 92412c2

dew326 · 2019-01-30T13:43:25Z

src/bundle/Resources/views/parts/tab/content_type.html.twig

@@ -17,7 +17,7 @@
                {% block page_title %}
                    {% include '@ezdesign/parts/page_title.html.twig' with {
                        title: 'content_type.view.view.title'|trans({ '%name%': content_type.name })|desc('%name%'),
-                        iconName: 'content-type'
+                        contentType: content_type.identifier


content_type_identifier?

webhdx · 2019-01-30T14:02:45Z

src/lib/Tab/Dashboard/PagerContentToDataMapper.php

@@ -76,6 +76,8 @@ public function map(Pagerfanta $pager): array
                'language' => $contentInfo->mainLanguageCode,
                'contributor' => $contributor,
                'version' => $content->versionInfo->versionNo,
+                'type' => $content->getContentType()->getName(),
+                'content_type' => $content->getContentType(),


What's the point of having these two? content_type is sufficient if this goes to the twig.

It is @adamwojs part, but I think that it is my error made during rebasing.

Fixed in 5e51a7e

barbaragr

Missing icons for:

Sub-items list - grid view

Dashboard/My scheduled:

Dashboard/Drafts under review:

Dashboard/Review queue

Dashboard/All scheduled:

Multi file upload after drag and drop:

barbaragr · 2019-02-01T15:48:10Z

Everything was fixed, just MFU left. It occurs when user tries to upload items when folder is empty - uploading for the first time. Later everything works fine.

tischsoic · 2019-02-04T15:02:52Z

src/bundle/Resources/public/js/scripts/admin.location.view.js

@@ -113,6 +113,13 @@
            return total;
        }, {});
        const udwConfigBulkMoveItems = JSON.parse(container.dataset.udwConfigBulkMoveItems);
+        const mfuContentTypesMap = Object.values(eZ.adminUiConfig.contentTypes).reduce((contentTypeDataMap, contentTypeGroup) => {


We need to pass different config to MFU because the one based on: container.dataset.contentTypes contains only CTs of elements displayed in Sub-items.

Given that mfuContentTypesMap in contrast to contentTypesMap (which is passed to Sub-items) does not contain whole CT objects but rather data stored in eZ.adminUiConfig.contentTypes, are we OK with passing mfuContentTypesMap to MFU as contentTypesMap - ping @dew326 @sunpietro ?

I think the approach is ok.

Is a difference only in missing data or in the data structure as well?

contentTypesMap is a map of CT objects; mfuContentTypesMap in this case is a map of objects like:

{ href: "/api/ezp/v2/content/types/21" identifier: "about" name: "About" thumbnail: "/bundles/ezplatformadminui/img/ez-icons.svg#about" }

name and thumbnail does not exist on CT object.
We can always call it contentTypesDataMap etc. We can also resign from passing this as a prop and extract it inside MFU. Or even make global helper function - getContentTypeIdentifierByHref, which may be helpful in other React components.

Thanks for explanation. Can't the backend provide this structure instead?

We opted for this solution mostly because this is more efficient than extracting objects for all Content Types. But maybe it is way to go. This question is more to @adamwojs .

sunpietro · 2019-02-05T11:06:16Z

src/bundle/Resources/public/js/scripts/admin.location.view.js

@@ -113,6 +113,13 @@
            return total;
        }, {});
        const udwConfigBulkMoveItems = JSON.parse(container.dataset.udwConfigBulkMoveItems);
+        const mfuContentTypesMap = Object.values(eZ.adminUiConfig.contentTypes).reduce((contentTypeDataMap, contentTypeGroup) => {


Thanks for explanation. Can't the backend provide this structure instead?

…ezsystems#805) * EZP-29999: Implement custom icons for content types * Change ezplatform_admin_ui_content_type_icon to ez_content_type_icon * Fix * contentType -> content_type_identifier * fixup! EZP-29999: Implement custom icons for content types * fixup! EZP-29999: Implement custom icons for content types * Pass different contentTypesMap to MFU

tischsoic self-assigned this Jan 17, 2019

tischsoic added Improvement Work in progress labels Jan 21, 2019

tischsoic mentioned this pull request Jan 21, 2019

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

Merged

tischsoic force-pushed the EZP-29999-as-editor-i-want-custom-icons-for-content-types-2 branch 2 times, most recently from 07cd74a to 6651a4a Compare January 21, 2019 15:07

sunpietro suggested changes Jan 24, 2019

View reviewed changes

EZP-29999: Implement custom icons for content types

600496c

tischsoic force-pushed the EZP-29999-as-editor-i-want-custom-icons-for-content-types-2 branch from 98f8197 to 600496c Compare January 30, 2019 12:00

Change ezplatform_admin_ui_content_type_icon to ez_content_type_icon

30bef37

tischsoic changed the title ~~[WIP] EZP-29999: As an editor, I want to see custom icons for content types~~ EZP-29999: As an editor, I want to see custom icons for content types Jan 30, 2019

tischsoic requested review from mikadamczyk, sunpietro, dew326, Nattfarinn, pawbuj, ViniTou and webhdx January 30, 2019 12:19

tischsoic assigned adamwojs Jan 30, 2019

tischsoic added Ready for review and removed Work in progress labels Jan 30, 2019

dew326 requested changes Jan 30, 2019

View reviewed changes

Fix

92412c2

dew326 requested changes Jan 30, 2019

View reviewed changes

contentType -> content_type_identifier

a3943c6

dew326 approved these changes Jan 30, 2019

View reviewed changes

webhdx reviewed Jan 30, 2019

View reviewed changes

fixup! EZP-29999: Implement custom icons for content types

5e51a7e

sunpietro approved these changes Jan 31, 2019

View reviewed changes

webhdx approved these changes Jan 31, 2019

View reviewed changes

lserwatka added the Ready for QA label Jan 31, 2019

barbaragr self-assigned this Jan 31, 2019

barbaragr requested changes Feb 1, 2019

View reviewed changes

barbaragr removed the Ready for QA label Feb 1, 2019

adamwojs and others added 2 commits February 4, 2019 13:33

fixup! EZP-29999: Implement custom icons for content types

abc1ece

Pass different contentTypesMap to MFU

a39e8b3

tischsoic commented Feb 4, 2019

View reviewed changes

sunpietro suggested changes Feb 5, 2019

View reviewed changes

sunpietro approved these changes Feb 5, 2019

View reviewed changes

barbaragr approved these changes Feb 5, 2019

View reviewed changes

sunpietro merged commit e1682c2 into master Feb 5, 2019

sunpietro deleted the EZP-29999-as-editor-i-want-custom-icons-for-content-types-2 branch February 5, 2019 14:49

barbaragr added the QA approved label Feb 5, 2019

m-tyrala mentioned this pull request Feb 6, 2019

EZP-29999 behat tests followup #832

Merged

2 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

tischsoic commented Jan 17, 2019 •

edited

Loading

sunpietro Jan 24, 2019

adamwojs Jan 28, 2019

sunpietro Jan 24, 2019

sunpietro Jan 24, 2019

sunpietro Jan 24, 2019

sunpietro Jan 24, 2019

sunpietro Jan 24, 2019

sunpietro Jan 24, 2019

tischsoic Jan 24, 2019 •

edited

Loading

tischsoic Jan 24, 2019

adamwojs Jan 28, 2019

tischsoic commented Jan 30, 2019

dew326 Jan 30, 2019

tischsoic Jan 30, 2019 •

edited

Loading

dew326 Jan 30, 2019

tischsoic Jan 30, 2019

dew326 Jan 30, 2019

tischsoic Jan 30, 2019

dew326 Jan 30, 2019

tischsoic Jan 30, 2019

dew326 Jan 30, 2019

webhdx Jan 30, 2019

tischsoic Jan 30, 2019

adamwojs Jan 31, 2019

barbaragr left a comment

barbaragr commented Feb 1, 2019

tischsoic Feb 4, 2019

dew326 Feb 5, 2019

sunpietro Feb 5, 2019

tischsoic Feb 5, 2019

sunpietro Feb 5, 2019

tischsoic Feb 5, 2019

sunpietro Feb 5, 2019

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

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

Conversation

tischsoic commented Jan 17, 2019 • edited Loading

Requires: #804

Views with icons

Content item view & edit view

Bookmarks

Trash

Drafts

Content type list; content type view; content type edit

Dashboard

Checklist:

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

tischsoic Jan 24, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tischsoic commented Jan 30, 2019

Choose a reason for hiding this comment

tischsoic Jan 30, 2019 • edited Loading

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

Choose a reason for hiding this comment

barbaragr left a comment

Choose a reason for hiding this comment

barbaragr commented Feb 1, 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

tischsoic commented Jan 17, 2019 •

edited

Loading

tischsoic Jan 24, 2019 •

edited

Loading

tischsoic Jan 30, 2019 •

edited

Loading