Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
base: 1.5
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
Changes from 6 commits
7a97f04
c40b66c
8b3687b
9e8739d
2a38bb2
0a812c4
e95e1d9
82a77c2
419b3fd
6936bb5
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
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:
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
There was a problem hiding this comment.
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
andEzBtnEmbed.isDisabled
was added: 8b3687bThere was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.