EZP-30732: As an Editor I want be able to add embed/images inside table cells #1034

SerheyDolgushev · 2019-07-08T13:08:03Z

Question	Answer
Tickets	EZP-30732
Bug fix?	no
New feature?	yes
BC breaks?	no
Tests pass?	yes
Doc needed?	no
License	GPL-2.0

Right not you can add ezembed and ezimage buttons into table/td toolbar:

alloy_editor:
    extra_buttons:
        td: [ezembed, ezimage]

But they are disabled.

Checklist:

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

dew326

Thanks for your contribution.

As we agreed to merge it into 2.5.x please:

Change the base to 1.5
Rebase branch with 1.5
Change the commit message to "EZP-30732: As an Editor, I want to be able to add embed/images inside table cells"

dew326 · 2019-07-09T08:09:29Z

src/bundle/Resources/public/js/alloyeditor/src/plugins/ez-embed.js

-                    const path = editor.elementPath();
-
-                    return !path || path.contains('table', true) === null;
+                    return true;


This method is not needed anymore so we can remove it in 3.0 for BC we need to keep it in 2.5.x.
Please add the deprecation information:

console.warn('[DEPRECATED] canBeAdded method is deprecated'); console.warn('[DEPRECATED] it will be removed from ezplatform-admin-ui 2.0');

The usage of this method can be safely removed from our buttons:
https://github.com/ezsystems/ezplatform-admin-ui/blob/master/src/bundle/Resources/public/js/alloyeditor/src/buttons/ez-btn-image.js#L60 and https://github.com/ezsystems/ezplatform-admin-ui/blob/master/src/bundle/Resources/public/js/alloyeditor/src/buttons/ez-btn-embed.js#L58
Please remove the disable state and the method isDisabled

All requested changes are done. Also a deprecation warning for EzBtnImage.isDisabled and EzBtnEmbed.isDisabled was added: 8b3687b

…le cells

…ated

barbaragr

Should I be allowed to put in one cell embed and image? Because embed/image buttons appear only when cell is empty or filled with text. I can't put right now embed item and image at one cell - different menu bar: http://g.recordit.co/b6GjiklyR4.gif What is more I can't do any operations (add row/remove column etc.) because of that.
Validation of XML
a. Click Create and select Article
b. Put a table 3x3 into Intro field
c. Embed an item in every field, use this button

d. Add an image to every cell with added button:

e. Click Publish or Save or Preview

Actual result: embedded items disappear, message is displayed: Validation of XML content failed: Error in 0:0: Element section has extra content: informaltable

SerheyDolgushev · 2019-08-01T13:30:55Z

sorry, is it something I should have look at?

barbaragr · 2019-08-02T10:52:33Z

@SerheyDolgushev yes, I've mentioned two issues in comment above.
First one, if you embed an image, you can do nothing more with a table cell (add row/remove column etc.) it is blocked, I don't know if you are ok with that.
Second one, if I embed something and add an image it's impossible to save the content.
Please, take a look at those bugs.

…ases it makes XML invalid

SerheyDolgushev · 2019-08-13T10:51:01Z

Should I be allowed to put in one cell embed and image? Because embed/image buttons appear only when cell is empty or filled with text. I can't put right now embed item and image at one cell - different menu bar: http://g.recordit.co/b6GjiklyR4.gif What is more I can't do any operations (add row/remove column etc.) because of that.

Please note, there are different toolbars, depending on the context of the current element in the editor. For example, if you are editing a table cell - you will see table cell toolbar. But if you are editing some text - you will see paragraph toolbar, even if the text is inside the table cell.
Before you will check it next time, please make sure you have the following settings on your local installation:

alloy_editor:
    extra_buttons:
        paragraph: [ezembed, ezimage]
        tr: [ezembed, ezimage]
        td: [ezembed, ezimage]

It will add embed and image buttons to listed toolbars.

Also please note, there two different types of embeds: inline embed and regular embed. And inline embed is available in most toolbars, by default.

So now you should be able to put embeds and images into the paragraph. But please note, there seems to be a separate bug: when you insert an embed or image in the paragraph in the table cell, it is added at the end of the edited field. I guess it might be fixed later?

Validation of XML

Initially, I thought there is an issue in the schema: ezsystems/ezplatform-richtext#56. But later it was discovered that this issue is caused by "empty width space", which was used in inline embeds. 9e8739d fixes it.

@barbaragr can you please make sure that extra_buttons are updated on your local installation and that it includes 9e8739d and test one more time.

barbaragr

But please note, there seems to be a separate bug: when you insert an embed or image in the paragraph in the table cell, it is added at the end of the edited field. I guess it might be fixed later?

It should be fix in this PR. I'm not able to embed two images or image and embed item in one cell, because, as you've mentioned, the second item is added at the end of the editing field (video for future retests: http://g.recordit.co/vg8OcHOAf3.gif). AFAIR it was working with this PR wich was closed ezsystems/ezplatform-richtext#56

SerheyDolgushev · 2019-08-14T13:36:42Z

But please note, there seems to be a separate bug: when you insert an embed or image in the paragraph in the table cell, it is added at the end of the edited field. I guess it might be fixed later?

@dew326 is it something you can have a look? Maybe there is an easy fix?

dew326 · 2019-08-14T13:49:16Z

src/bundle/Resources/public/js/alloyeditor/src/plugins/ez-embed.js

@@ -64,7 +64,6 @@ const ZERO_WIDTH_SPACE = '&#8203;';
                },

                insertWrapper: function(wrapper) {
-                    this.editor.insertHtml(ZERO_WIDTH_SPACE);


This was the fix for https://jira.ez.no/browse/EZP-29882
I think we cannot merge this.

@dew326 sorry, but this line causes errors: in some cases, the generated XML is invalid because of zero-width space. I tried to reproduce the mentioned issue and wasn't able to do so. Seems like inline embed functionality changed: user don't provide any text. @barbaragr can you please check this one also?

@dew326 is right, regression could occur if we merged it: https://recordit.co/gtMfFuBtyl

https://jira.ez.no/browse/EZP-29882 doesn't work here.

dew326 · 2019-08-14T13:50:40Z

@SerheyDolgushev Sorry, in 10 minutes I have days off to the end of the week. I would start looking from here: https://github.com/ezsystems/ezplatform-admin-ui/blob/master/src/bundle/Resources/public/js/alloyeditor/src/plugins/ez-add-content.js#L57

SerheyDolgushev · 2019-08-15T12:43:40Z

@dew326 thanks for pointing me towards the correct direction.
@barbaragr please note that latest commit fixes the bug which didn't allow to insert the embed into the table cell paragraph.

barbaragr

Some new issues discovered:

Adding image to the list -> http://g.recordit.co/jzciHmG1qX.gif
When I add an image, it's added below the list, not in a proper position.
In a table it's not possible to add next element to the list, while there is an image/embedded item -> http://g.recordit.co/jLsIMTXn6e.gif
When I add an image to the list in table, I can't add next element. It is possible to add something only above the image/embedded item.
Image added to the list in table, after publishing goes above the list -> http://g.recordit.co/j40GxtHbaT.gif
a. Add a table
b. Add a list with 3 strings
c. In the middle of the list add another element - an image
d. Publish the content
e. Noticed that image in preview went up, above the list
f. Go to edit mode - image is still on the top of the cell
If you add two images into table cell there is an empty gap between them and that's the only way to add next images - user can't add them ad the end, only between those two. The same occurs for images on the list. Is it the only way to have a possibility to add more than one image? https://recordit.co/UPvsAVas45

SerheyDolgushev · 2019-08-23T10:35:19Z

@barbaragr nested elements for the list is out of scope for this PR. So when you try to add a new element (embed/image/custom tag/etc) in the list item, it will be added after the list. Which is pretty close to the default behavior. It fixes issues 1-3 you mentioned earlier. Let's have a separate PR for nested list item elements. @SylvainGuittard

Regarding issue 4, there is an empty paragraph between two images. And you can remove it using trash button:

If everything looks correct, the only remaining issue is to fix https://jira.ez.no/browse/EZP-29882. Please note, the original fix was not the best way to deal with it, as in some cases it leads to invalid XML in the Richt Text field (because of zero-width spaces). @dew326 do you think there should be another fix, which doesn't modify the content of the XML?

dew326 · 2019-08-23T11:32:59Z

Yes, we should have a fix for this ticket. We cannot merge bugfix which will make a regression on another issue. Currently, I don't have an idea of how to fix this without adding a space.

src/bundle/Resources/public/js/alloyeditor/src/plugins/ez-add-content.js

dew326 · 2019-08-27T05:59:37Z

src/bundle/Resources/public/js/alloyeditor/src/plugins/ez-add-content.js

+            const isCurrentElementList = element.getName() == 'ul' || element.getName() == 'ol';
+
+            let insertIndex = insideTableCell || insideCustomTag ? 0 : elements.length - 2;
+            if (elementPath.lastElement.getName() == 'li') {


And here I have a question why you have to check if this is a list item and increase the index

@dew326 Answering your question:
If you are trying to insert any content (embed/image/etc) into the list item, it will be added after the list element, but not the list item element. And it fixes list item related issues previously submitted by @barbaragr
I guess this behavior will be changed in the future when it will be allowed to insert embed/images/etc inside list items.

SerheyDolgushev · 2019-08-28T08:13:00Z

@dew326 e95e1d9 implements all your suggestions. Please have a look.

barbaragr · 2019-08-28T14:28:28Z

Apart from https://jira.ez.no/browse/EZP-29882 again occurs an issue from my very first comment point 2.

Validation of XML
a. Click Create and select Article
b. Put a table 3x3 into Intro field
c. Embed an item in every field, use this button

d. Add an image to every cell with added button:

e. Click Publish or Save or Preview

Actual result: embedded items disappear, message is displayed: Validation of XML content failed: Error in 0:0: Element section has extra content: informaltable

@SerheyDolgushev according to:

@barbaragr nested elements for the list is out of scope for this PR. So when you try to add a new element (embed/image/custom tag/etc) in the list item, it will be added after the list. Which is pretty close to the default behavior. It fixes issues 1-3 you mentioned earlier. Let's have a separate PR for nested list item elements. @SylvainGuittard

Of course I can report it separately as follow up, but on this branch, right now, when you want to add an image in the middle of the list, it is added under the field: http://g.recordit.co/wcTT0Pz0kU.gif which is different than default behaviour - adding the image under the list.

Regarding issue 4, there is an empty paragraph between two images. And you can remove it using trash button:

I didn't ask about removing the paragraph, I'm familiar with trash button. The point is, when you add two images, there is an empty paragraph between them and it is the only way to add more images. If you remove the paragraph you can't add more images to the cell.

…able

SerheyDolgushev · 2020-03-10T10:25:34Z

In the latest commit embed and image buttons have been added to the table cell toolbar.
Please test this PR using thttps://github.com/ezsystems/ezplatform/releases/tag/v2.5.9

dew326 · 2020-03-10T11:14:43Z

I tested this briefly.
First what I tested is:

Adding image to the list -> http://g.recordit.co/jzciHmG1qX.gif
When I add an image, it's added below the list, not in a proper position.

And here is the result:

I didn't test the rest of the issues.

SerheyDolgushev · 2020-03-13T10:06:14Z

Sorry, but images inside the lists are out of scope for this PR. Can you please test how the images/embeds are functionating inside the table cells? And probably we should have a separate ticket/PR for lists?

dew326 · 2020-03-13T12:35:24Z

@SerheyDolgushev sorry but it was already explained why we cannot merge it like this.

Of course I can report it separately as follow up, but on this branch, right now, when you want to add an image in the middle of the list, it is added under the field: http://g.recordit.co/wcTT0Pz0kU.gif which is different than default behaviour - adding the image under the list.

Without your changes, the image is added below the list, but with your changes it's added below the whole richtext field. We cannot merge it like this.

…ix lists

SerheyDolgushev · 2020-03-17T15:03:42Z

@dew326 List item issue is fixed. So now when the image is added into the list item, it will be added after the list. We can change the UI to insert the image inside the list item. But it would require some changes in the schema. As right now schema does not let to store images inside list items.
Please let me know if you will find any other issues, which prevent this PR to be merged.

dew326

Just minor issues. We will send it to the QA.

dew326 · 2020-03-18T09:49:03Z

src/bundle/Resources/public/js/alloyeditor/src/plugins/ez-add-content.js

-            const insertIndex = !elementPath.contains(isCustomTag, true) ? elements.length - 2 : 0;
+            const insideTableCell = elementPath.blockLimit && elementPath.blockLimit.getName() === 'td';
+            const insideCustomTag = elementPath.contains(isCustomTag, true);
+            const insideListItem = elementPath.lastElement.getName() == 'li';


Suggested change

const insideListItem = elementPath.lastElement.getName() == 'li';

const insideListItem = elementPath.lastElement.getName() === 'li';

dew326 · 2020-03-18T09:49:14Z

src/bundle/Resources/public/js/alloyeditor/src/plugins/ez-add-content.js

+            const insideListItem = elementPath.lastElement.getName() == 'li';
+            let insertIndex = insideTableCell || insideCustomTag || insideListItem ? 0 : elements.length - 2;
+
+            if (elementPath.lastElement.getName() == 'li') {


Suggested change

if (elementPath.lastElement.getName() == 'li') {

if (elementPath.lastElement.getName() === 'li') {

katarzynazawada

@SerheyDolgushev , issue mentioned by @barbaragr still occurs

Validation of XML

a. Click Create and select Article
b. Put a table 3x3 into Intro field
c. Embed an item in every field, use this button

d. Add an image to every cell with added button:

e. Click Publish or Save or Preview

Actual result: embedded items disappear, message is displayed: Validation of XML content failed: Error in 0:0: Element section has extra content: informaltable

SerheyDolgushev · 2020-12-08T14:36:01Z

I guess this one should be closed in favor of ezsystems/ezplatform-richtext#171. @dew326 can you please confirm it?

dew326 · 2020-12-08T14:42:01Z

I think the goal here was to enable this in 2.5.

As far as I know, a customer wanted this? @SylvainGuittard, @konradoboza

konradoboza · 2020-12-08T14:47:19Z

I can confirm that.

SylvainGuittard · 2020-12-08T15:26:57Z

Yes, we should have this feature on 2.5

dew326 reviewed Jul 9, 2019

View reviewed changes

EZP-30732: As an Editor I want be able to add embed/images inside tab…

7a97f04

…le cells

SerheyDolgushev changed the base branch from master to 1.5 July 9, 2019 08:32

SerheyDolgushev force-pushed the fix_allow_embed_in_table branch from 08059f5 to 7a97f04 Compare July 9, 2019 08:32

SerheyDolgushev changed the title ~~Allow embeds in inside tables~~ EZP-30732: As an Editor I want be able to add embed/images inside table cells Jul 9, 2019

SerheyDolgushev added 2 commits July 9, 2019 09:37

EZP-30732: Deprecation warning for editor.ezembed.canBeAdded

c40b66c

EZP-30732: EzBtnImage.isDisabled and EzBtnEmbed.isDisabled are deprec…

8b3687b

…ated

dew326 approved these changes Jul 9, 2019

View reviewed changes

barbaragr requested changes Jul 10, 2019

View reviewed changes

barbaragr self-assigned this Jul 10, 2019

SerheyDolgushev mentioned this pull request Aug 7, 2019

EZP-30732: Allows table cells to contain inlines and blocks ezsystems/ezplatform-richtext#56

Closed

4 tasks

This comment has been minimized.

Sign in to view

EZP-30732: Dont use empty width space for inline embeds, as in some c…

9e8739d

…ases it makes XML invalid

barbaragr requested changes Aug 14, 2019

View reviewed changes

dew326 requested changes Aug 14, 2019

View reviewed changes

EZP-30732: allow to inser embeds inside table cell paragraph

2a38bb2

barbaragr requested changes Aug 20, 2019

View reviewed changes

EZP-30732: disable child elements in the lists

0a812c4

dew326 reviewed Aug 27, 2019

View reviewed changes

EZP-30732: CS fixes suggested by Dariusz

e95e1d9

SerheyDolgushev mentioned this pull request Dec 2, 2019

EZP-31321: Replaced zero-width space with oe-upcast-placeholder ezsystems/ezplatform-richtext#88

Merged

4 tasks

SerheyDolgushev added 2 commits March 10, 2020 10:13

Merge remote-tracking branch 'upstream/1.5' into fix_allow_embed_in_t…

82a77c2

…able

EZP-30732: embed and image buttons are added to table-cell toolbar

419b3fd

EZP-30732: embed and image buttons are added to table-cell toolbar, f…

6936bb5

…ix lists

SerheyDolgushev requested a review from dew326 March 17, 2020 15:03

dew326 reviewed Mar 18, 2020

View reviewed changes

katarzynazawada self-assigned this Mar 19, 2020

katarzynazawada requested changes Mar 19, 2020

View reviewed changes

mateuszdebinski pushed a commit that referenced this pull request Feb 13, 2024

Fixed PHPStan issues (#1034)

d01511a

	const insideListItem = elementPath.lastElement.getName() == 'li';
	const insideListItem = elementPath.lastElement.getName() === 'li';

	if (elementPath.lastElement.getName() == 'li') {
	if (elementPath.lastElement.getName() === 'li') {

EZP-30732: As an Editor I want be able to add embed/images inside table cells #1034

Are you sure you want to change the base?

EZP-30732: As an Editor I want be able to add embed/images inside table cells #1034

Conversation

SerheyDolgushev commented Jul 8, 2019 • edited Loading

Checklist:

dew326 left a comment • edited Loading

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

SerheyDolgushev commented Aug 1, 2019

barbaragr commented Aug 2, 2019

This comment has been minimized.

SerheyDolgushev commented Aug 13, 2019 • edited Loading

barbaragr left a comment • edited Loading

Choose a reason for hiding this comment

SerheyDolgushev commented Aug 14, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

barbaragr Aug 19, 2019 • edited Loading

Choose a reason for hiding this comment

dew326 commented Aug 14, 2019

SerheyDolgushev commented Aug 15, 2019

barbaragr left a comment

Choose a reason for hiding this comment

SerheyDolgushev commented Aug 23, 2019

dew326 commented Aug 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SerheyDolgushev commented Aug 28, 2019

barbaragr commented Aug 28, 2019 • edited Loading

SerheyDolgushev commented Mar 10, 2020

dew326 commented Mar 10, 2020

SerheyDolgushev commented Mar 13, 2020

dew326 commented Mar 13, 2020

SerheyDolgushev commented Mar 17, 2020

dew326 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katarzynazawada left a comment

Choose a reason for hiding this comment

SerheyDolgushev commented Dec 8, 2020

dew326 commented Dec 8, 2020

konradoboza commented Dec 8, 2020

SylvainGuittard commented Dec 8, 2020

SerheyDolgushev commented Jul 8, 2019 •

edited

Loading

dew326 left a comment •

edited

Loading

SerheyDolgushev commented Aug 13, 2019 •

edited

Loading

barbaragr left a comment •

edited

Loading

barbaragr Aug 19, 2019 •

edited

Loading

barbaragr commented Aug 28, 2019 •

edited

Loading