EZP-31236: Added tooltips as new UI component #1176

lucasOsti · 2019-12-27T12:33:55Z

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

Added tooltips as new UI Component. Usage like bootstrap tooltip but ez-tooltip have 3 extra parameters added to tooltip caller node:

data-extra-classes - additional class added to tooltip node .tooltip
data-delay-show - show delay
data-delay-hide - hide delay

All PR’s to this ticket:
ezsystems/ezplatform-matrix-fieldtype#30
https://github.com/ezsystems/ezplatform-form-builder/pull/205
ezsystems/ezplatform-user#58
https://github.com/ezsystems/ezplatform-workflow/pull/106
https://github.com/ezsystems/date-based-publisher/pull/148
ezsystems/ezplatform-admin-ui-modules#246
#1176
https://github.com/ezsystems/ezplatform-page-builder/pull/499

Checklist:

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

src/bundle/Resources/public/scss/core/_custom.tooltip.scss

GrabowskiM · 2019-12-27T13:09:51Z

src/bundle/Resources/public/scss/core/_custom.tooltip.scss

+        color: $ez-black;
+        font-weight: 700;
+        text-align: left;
+        box-shadow: 0 4px 6px rgba(135,135,135,0.35);


calculateRem

lucasOsti · 2019-12-27T14:16:38Z

GrabowskiM · 2020-01-03T11:17:19Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+(function(doc) {
+    jQuery(document).ready(() => {
+        const tooltipsNode = doc.querySelectorAll('[data-toggle="tooltip"]');
+        let delay, extraClass;


You can always declare it as consts inside for loop

inakijv · 2020-01-08T19:34:57Z

Thank you for this contribution @lucasOsti 👍, it is going to improve the UX of the application. Could you please add images of the two different tooltips? (maybe also a gif would be good too)

dew326 · 2020-01-10T09:19:19Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

@@ -0,0 +1,22 @@
+(function(doc) {
+    jQuery(document).ready(() => {


We don't do it in any other places so we also shouldn't do it here.

dew326 · 2020-01-10T09:19:58Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

@@ -0,0 +1,22 @@
+(function(doc) {


Suggested change

(function(doc) {

(function(global, doc, $,) {

dew326 · 2020-01-10T09:20:18Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+            });
+        }
+    });
+})(window.document);


Suggested change

})(window.document);

})(window, window.document, window.jQuery);

dew326 · 2020-01-10T09:20:33Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+            };
+            extraClass = tooltipNode.getAttribute('data-extra-class') || '';
+
+            jQuery(tooltipNode).tooltip({


Suggested change

jQuery(tooltipNode).tooltip({

$(tooltipNode).tooltip({

dew326 · 2020-01-10T09:21:16Z

src/bundle/Resources/public/scss/core/_custom.tooltip.scss

+        color: $ez-black;
+        font-weight: 700;
+        text-align: left;
+        box-shadow: 0 calculateRem(4px) calculateRem(6px) rgba(135,135,135,0.35);


Please use Prettier

dew326 · 2020-01-10T09:23:24Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+                show: tooltipNode.getAttribute('data-delay-show') || 150,
+                hide: tooltipNode.getAttribute('data-delay-hide') || 75,
+            };
+            extraClass = tooltipNode.getAttribute('data-extra-class') || '';


I would name it extra-classes to be consistent with other places and this may contain multiple classes

dew326 · 2020-01-10T09:23:40Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+            extraClass = tooltipNode.getAttribute('data-extra-class') || '';
+
+            jQuery(tooltipNode).tooltip({
+                delay: delay,


Suggested change

delay: delay,

delay,

dew326 · 2020-01-10T09:24:05Z

src/bundle/Resources/public/scss/core/_custom.tooltip.scss

+
+    &--big &__inner {
+        padding: calculateRem(8px) calculateRem(16px); 
+        font-size: calculateRem(13px);


Default is 16px and big is 13px.

dew326 · 2020-01-10T09:24:51Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

@@ -0,0 +1,22 @@
+(function(doc) {
+    jQuery(document).ready(() => {
+        const tooltipsNode = doc.querySelectorAll('[data-toggle="tooltip"]');


This will not work on our all tooltips with title

dew326 · 2020-01-10T13:00:08Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

@@ -0,0 +1,28 @@
+(function(global, doc, eZ, $) {
+    eZ.helpers.tooltips = {


Please follow the convention of other helpers.
const with method
using the eZ.addConfig and so

dew326 · 2020-01-10T13:00:25Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+    eZ.helpers.tooltips = {
+        parse: () => {
+            const tooltipsNode = doc.querySelectorAll('[title]');
+            let delay, extraClasses;


Can be done as const inside the if

dew326 · 2020-01-10T13:02:06Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+                    $(tooltipNode).tooltip({
+                        delay,
+                        template: `<div class="tooltip ez-tooltip ${extraClasses}">
+                                            <div class="arrow ez-tooltip__arrow"></div>


Indentation is not the best here

dew326 · 2020-01-10T13:03:37Z

src/bundle/Resources/public/js/scripts/core/custom.tooltip.js

+        },
+    };
+
+    eZ.helpers.tooltips.parse();


Not sure if helper itself should invoke the method.
The helper should be included with other helpers but invoked in the other file so we could move it as needed.

lucasOsti · 2020-01-13T09:42:10Z

Thank you for this contribution @lucasOsti 👍, it is going to improve the UX of the application. Could you please add images of the two different tooltips? (maybe also a gif would be good too)

src/lib/Menu/ContentCreateRightSidebarBuilder.php

src/lib/Menu/ContentEditRightSidebarBuilder.php

src/lib/Menu/ContentRightSidebarBuilder.php

src/lib/Menu/LeftSidebarBuilder.php

adamwojs

+1 for the changes in *.php (besides code style issues)

adamwojs · 2020-01-15T08:20:47Z

src/bundle/Resources/public/js/scripts/helpers/tooltips.helper.js

+                    show: tooltipNode.getAttribute('data-delay-show') || 150,
+                    hide: tooltipNode.getAttribute('data-delay-hide') || 75,
+                };
+                const extraClasses = tooltipNode.getAttribute('data-extra-classes') || '';


Probably could be replaced by tooltipNode.dataset.extraClasses. Ref. https://developer.mozilla.org/en-US/docs/Learn/HTML/Howto/Use_data_attributes

ViniTou · 2020-01-15T08:27:13Z

But aside from CS issues, behat test were green two days ago on master, so you should look at them too. Maybe rebase would help.

mnocon · 2020-01-15T09:18:37Z

The HTML has changed in some places. Comparison of two elements where tests fail:

Before:

<a title="Assign to Users/Groups" href="/admin/role/1/assignment/create" class="btn btn-icon mx-3">
<a href="/admin/role/1/assignment/create" title="Assign to Users/Groups" class="btn btn-secondary">

After:

<a href="/admin/role/1/assignment/create" title="" class="btn btn-secondary" data-original-title="Assign to Users/Groups">
<a title="" href="/admin/role/1/assignment/create" class="btn btn-icon mx-3" data-original-title="Assign to Users/Groups">

The value previously found in title attribute is now in data-original-title. @lucasOsti please let me know if this is ok, if it is I will adjust the selectors (there are some similar to .ez-table-header [title^=Assign], which cannot find the correct elements right now).

lucasOsti · 2020-01-15T12:24:42Z

The HTML has changed in some places. Comparison of two elements where tests fail:

Before:
<a title="Assign to Users/Groups" href="/admin/role/1/assignment/create" class="btn btn-icon mx-3">
<a href="/admin/role/1/assignment/create" title="Assign to Users/Groups" class="btn btn-secondary">
After:
<a href="/admin/role/1/assignment/create" title="" class="btn btn-secondary" data-original-title="Assign to Users/Groups">
<a title="" href="/admin/role/1/assignment/create" class="btn btn-icon mx-3" data-original-title="Assign to Users/Groups">
The value previously found in title attribute is now in data-original-title. @lucasOsti please let me know if this is ok, if it is I will adjust the selectors (there are some similar to .ez-table-header [title^=Assign], which cannot find the correct elements right now).

Now every title tag is changed to data-original-title by bootstrap javascript

katarzynazawada

Header in Form Builder is misaligned.
UDW does not work. Cannot select any location, UDW is empty
https://res.cloudinary.com/ezplatformtravis/image/upload/v1581073005/screenshots/5e3d426d840c7664459593-vendor_ezsystems_ezplatform-admin-ui_features_personas_subtreeeditor_feature_41_v5ebyr.png
Error occurs Cannot read property 'Location' of undefined
Widgets are not properly styled - Publish Later/Send to review. Also in Page Builder.

lucasOsti · 2020-02-07T13:44:50Z

I made rebase, everything should be ok

lucasOsti requested review from GrabowskiM and dew326 December 27, 2019 12:33

GrabowskiM requested changes Dec 27, 2019

View reviewed changes

lucasOsti requested a review from GrabowskiM December 27, 2019 14:14

GrabowskiM approved these changes Jan 3, 2020

View reviewed changes

dew326 requested changes Jan 10, 2020

View reviewed changes

lucasOsti requested review from dew326 and GrabowskiM January 10, 2020 12:55

dew326 requested changes Jan 10, 2020

View reviewed changes

lucasOsti requested review from dew326 and adamwojs and removed request for adamwojs January 13, 2020 09:51

dew326 requested review from webhdx, ViniTou, adamwojs and Nattfarinn January 14, 2020 07:56

GrabowskiM requested changes Jan 14, 2020

View reviewed changes

src/lib/Menu/ContentCreateRightSidebarBuilder.php Show resolved Hide resolved

src/lib/Menu/ContentEditRightSidebarBuilder.php Show resolved Hide resolved

src/lib/Menu/ContentRightSidebarBuilder.php Show resolved Hide resolved

src/lib/Menu/LeftSidebarBuilder.php Show resolved Hide resolved

lucasOsti requested a review from GrabowskiM January 14, 2020 09:38

GrabowskiM approved these changes Jan 14, 2020

View reviewed changes

adamwojs approved these changes Jan 15, 2020

View reviewed changes

ViniTou approved these changes Jan 15, 2020

View reviewed changes

adamwojs changed the title ~~EZP-31236 Added tooltips as new UI component~~ EZP-31236: Added tooltips as new UI component Jan 15, 2020

lucasOsti assigned mnocon Jan 15, 2020

lucasOsti added the Ready for QA label Jan 15, 2020

katarzynazawada requested changes Feb 7, 2020

View reviewed changes

lucasOsti requested a review from katarzynazawada February 7, 2020 13:44

dew326 approved these changes Feb 10, 2020

View reviewed changes

lucasOsti requested review from dew326 and GrabowskiM February 11, 2020 08:39

GrabowskiM approved these changes Feb 11, 2020

View reviewed changes

dew326 approved these changes Feb 11, 2020

View reviewed changes

lucasOsti requested a review from dew326 February 11, 2020 15:40

dew326 approved these changes Feb 12, 2020

View reviewed changes

lucasOsti and others added 15 commits February 21, 2020 11:43

EZP-31236 Restaled bootstrap tooltip

9f20003

removed empty line in scss file

4d94e8c

Added fixes after code review

4dde1fe

Added changes after code review

0a69030

Added changes after code review

ba01e0b

Added changes after code review

504aa98

[Behat] Adjusted selectors for changed element attributes

be249f5

Added title tags to action buttons

866b425

Fix CS build error

278675d

After code review

87b8606

After code review

8f1ab31

After code review

5b04bf5

Moved title parameter to ez-data-source in inputs

08c272a

after QA

e6ae6f5

Fixed hidden of tooltips

b58dbd3

lucasOsti force-pushed the EZP-31236-add-tooltips branch from b6c114a to b58dbd3 Compare February 21, 2020 10:47

katarzynazawada approved these changes Feb 24, 2020

View reviewed changes

katarzynazawada added QA approved and removed Ready for QA labels Feb 24, 2020

lserwatka merged commit 958e4e9 into ezsystems:master Feb 24, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

EZP-31236: Added tooltips as new UI component #1176

EZP-31236: Added tooltips as new UI component #1176

lucasOsti commented Dec 27, 2019 •

edited

Loading

GrabowskiM Dec 27, 2019

lucasOsti commented Dec 27, 2019

GrabowskiM Jan 3, 2020

inakijv commented Jan 8, 2020 •

edited

Loading

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

dew326 Jan 10, 2020

lucasOsti commented Jan 13, 2020

adamwojs left a comment •

edited

Loading

adamwojs Jan 15, 2020

ViniTou commented Jan 15, 2020

mnocon commented Jan 15, 2020

lucasOsti commented Jan 15, 2020

katarzynazawada left a comment

lucasOsti commented Feb 7, 2020 •

edited

Loading

		@@ -0,0 +1,22 @@
		(function(doc) {
		jQuery(document).ready(() => {

	})(window.document);
	})(window, window.document, window.jQuery);

		@@ -0,0 +1,28 @@
		(function(global, doc, eZ, $) {
		eZ.helpers.tooltips = {

EZP-31236: Added tooltips as new UI component #1176

EZP-31236: Added tooltips as new UI component #1176

Conversation

lucasOsti commented Dec 27, 2019 • edited Loading

Checklist:

Choose a reason for hiding this comment

lucasOsti commented Dec 27, 2019

Choose a reason for hiding this comment

inakijv commented Jan 8, 2020 • 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lucasOsti commented Jan 13, 2020

adamwojs left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ViniTou commented Jan 15, 2020

mnocon commented Jan 15, 2020

lucasOsti commented Jan 15, 2020

katarzynazawada left a comment

Choose a reason for hiding this comment

lucasOsti commented Feb 7, 2020 • edited Loading

lucasOsti commented Dec 27, 2019 •

edited

Loading

inakijv commented Jan 8, 2020 •

edited

Loading

adamwojs left a comment •

edited

Loading

lucasOsti commented Feb 7, 2020 •

edited

Loading