Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 10 commits into
base: 1.5
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export default class EzBtnEmbed extends EzEmbedDiscoverContentButton {
* @return {Boolean} True if the command is disabled, false otherwise.
*/
isDisabled() {
console.warn('[DEPRECATED] EzBtnEmbed.isDisabled method is deprecated');
console.warn('[DEPRECATED] it will be removed from ezplatform-admin-ui 2.0');
return !this.props.editor.get('nativeEditor').ezembed.canBeAdded();
}

Expand Down Expand Up @@ -55,13 +57,11 @@ export default class EzBtnEmbed extends EzEmbedDiscoverContentButton {
*/
render() {
const css = 'ae-button ez-btn-ae ez-btn-ae--embed ' + this.getStateClasses();
const disabled = this.isDisabled();
const label = Translator.trans(/*@Desc("Embed")*/ 'embed_btn.label', {}, 'alloy_editor');

return (
<button
className={css}
disabled={disabled}
onClick={this.chooseContent.bind(this)}
tabIndex={this.props.tabIndex}
title={label}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ export default class EzBtnImage extends EzEmbedImageButton {
* @return {Boolean} True if the command is disabled, false otherwise.
*/
isDisabled() {
console.warn('[DEPRECATED] EzBtnImage.isDisabled method is deprecated');
console.warn('[DEPRECATED] it will be removed from ezplatform-admin-ui 2.0');
return !this.props.editor.get('nativeEditor').ezembed.canBeAdded();
}

Expand Down Expand Up @@ -56,14 +58,12 @@ export default class EzBtnImage extends EzEmbedImageButton {
* @return {Object} The content which should be rendered.
*/
render() {
const css = 'ae-button ez-btn-ae ez-btn-ae--image ' + this.getStateClasses(),
disabled = this.isDisabled();
const css = 'ae-button ez-btn-ae ez-btn-ae--image ' + this.getStateClasses();
const label = Translator.trans(/*@Desc("Image")*/ 'image_btn.label', {}, 'alloy_editor');

return (
<button
className={css}
disabled={disabled}
onClick={this.chooseContent.bind(this)}
tabIndex={this.props.tabIndex}
title={label}>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,14 @@

if (elementPath && elementPath.block) {
const elements = elementPath.elements;
const insertIndex = !elementPath.contains(isCustomTag, true) ? elements.length - 2 : 0;
const insideTableCell = elementPath.blockLimit && elementPath.blockLimit.getName() == 'td';
dew326 marked this conversation as resolved.
Show resolved Hide resolved
const insideCustomTag = elementPath.contains(isCustomTag, true);
const isCurrentElementList = element.getName() == 'ul' || element.getName() == 'ol';
dew326 marked this conversation as resolved.
Show resolved Hide resolved

dew326 marked this conversation as resolved.
Show resolved Hide resolved
let insertIndex = insideTableCell || insideCustomTag ? 0 : elements.length - 2;
dew326 marked this conversation as resolved.
Show resolved Hide resolved
if (elementPath.lastElement.getName() == 'li') {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

insertIndex++;
}

element.insertAfter(elements[insertIndex]);
} else if (editor.widgets && editor.widgets.focused) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ const ZERO_WIDTH_SPACE = '&#8203;';
init: function(editor) {
editor.ezembed = {
canBeAdded: () => {
const path = editor.elementPath();

return !path || path.contains('table', true) === null;
console.warn('[DEPRECATED] canBeAdded method is deprecated');
console.warn('[DEPRECATED] it will be removed from ezplatform-admin-ui 2.0');
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

},
};

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

insertWrapper: function(wrapper) {
this.editor.insertHtml(ZERO_WIDTH_SPACE);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@barbaragr barbaragr Aug 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

this.editor.insertElement(wrapper);
},

Expand Down