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

[SerheyDolgushev]

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:

• [x] Coding standards ($ composer fix-cs)
• [x] Ready for Code Review

[SerheyDolgushev]

sorry, is it something I should have look at?

[barbaragr]

@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.

[SerheyDolgushev]

Depends on https://github.com/ezsystems/ezplatform-richtext/pull/56. We need to merge that PR first, to be able to continue work on this one.

[SerheyDolgushev]

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: https://github.com/ezsystems/ezplatform-richtext/pull/56. But later it was discovered that this issue is caused by "empty width space", which was used in inline embeds. https://github.com/ezsystems/ezplatform-admin-ui/pull/1034/commits/9e8739ddb889270b506d159bf396ae259e99f02a fixes it.

@barbaragr can you please make sure that extra_buttons are updated on your local installation and that it includes https://github.com/ezsystems/ezplatform-admin-ui/pull/1034/commits/9e8739ddb889270b506d159bf396ae259e99f02a and test one more time.

[SerheyDolgushev]

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]

@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]

@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.

[SerheyDolgushev]

@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]

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.

[SerheyDolgushev]

@dew326 https://github.com/ezsystems/ezplatform-admin-ui/pull/1034/commits/e95e1d99f60ac2492a39bb124c41ce47f0460461 implements all your suggestions. Please have a look.

[barbaragr]

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.

[SerheyDolgushev]

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]

I tested this briefly. First what I tested is:

1. 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]

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]

@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.

[SerheyDolgushev]

@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.

[SerheyDolgushev]

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

[dew326]

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

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

[konradoboza]

I can confirm that.

[SylvainGuittard]

Yes, we should have this feature on 2.5