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-29355: As a developer, I want to edit the ezcontent of a custom tag #697

Merged
merged 1 commit into from
Dec 10, 2018

Conversation

dew326
Copy link
Member

@dew326 dew326 commented Oct 19, 2018

Question Answer
Tickets https://jira.ez.no/browse/EZP-29355
Requires ezsystems/ezplatform-richtext#19
Bug fix? no
New feature? yes/
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

Checklist:

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

@dew326 dew326 self-assigned this Oct 19, 2018
@dew326 dew326 force-pushed the editable-ezcontent branch 2 times, most recently from 39a14e6 to 25629ef Compare October 19, 2018 09:45
const data = alloyEditor.get('nativeEditor').getData();
const doc = document.createDocumentFragment();
const root = document.createElement('div');
const ezNamespace = 'http://ez.no/namespaces/ezpublish5/xhtml5/edit';
const xhtmlNamespace = 'http://www.w3.org/1999/xhtml';
const emptyEmbed = function (embedNode) {
const emptyEmbed = function(embedNode) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be extracted outside of callback scope, should it?

embedNode.classList.remove(cl);
prevRemoveClass();
};
}
});
removeClass();
};
const xhtmlify = function (data) {
const clearCustomTag = (customTag) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be extracted outside of callback scope, should it?

attributesNodes.forEach((attributesNode) => attributesNode.remove());
headers.forEach((header) => header.remove());
};
const xhtmlify = function(data) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't it be extracted outside of callback scope, should it?

src/bundle/Resources/public/scss/_custom-tag.scss Outdated Show resolved Hide resolved
src/bundle/Resources/public/scss/_custom-tag.scss Outdated Show resolved Hide resolved
@dew326 dew326 force-pushed the editable-ezcontent branch from 25629ef to 921954d Compare October 22, 2018 09:42
@tischsoic
Copy link
Contributor

I think we should reformat all *.js files according to our Prettify rules. Reading further PRs without formatting changes would be a lot easier. :) ping @sunpietro @dew326

@dew326 dew326 force-pushed the editable-ezcontent branch from 921954d to 5f01495 Compare October 24, 2018 08:46
@micszo micszo assigned micszo and unassigned micszo Oct 24, 2018
@bdunogier
Copy link
Member

What is the status of this PR ? I can't really tell based on the latest contributions to it.

@dew326
Copy link
Member Author

dew326 commented Nov 6, 2018

@bdunogier This waits for QA, but we cannot test it without "nested custom tags".

@bdunogier
Copy link
Member

bdunogier commented Nov 7, 2018

Why does testing require nested custom tags, @dew326 ? If you test editing of regular text, without tags in the ezcontent, isn't it enough ?

Also, what is the status of nested custom tags, and/or jira story ?

@dew326
Copy link
Member Author

dew326 commented Nov 7, 2018

@micszo tried to test it but we couldn't determinate which problems are because of the ezcontent and which because of nested custom tags. For example, embed and embed image doesn't work because problems in nesting ezconfig (probably will be fixed in nested custom tags).

WIthout nested custom tags it's not possible to make full and proper QA, and probably the QA would need to be duplicated while testing nested custom tags.

@bdunogier
Copy link
Member

I understand.

On the other hand, we have customers waiting for this, in order to edit content that doesn't include nested tags. I really see benefits in having this feature without the nesting part. The scope would then be very limited, but it would still bring something valuable.

Would it be complicated to disable all the OE buttons when the ezcontent editing box has focus ?

@dew326
Copy link
Member Author

dew326 commented Nov 7, 2018

But the nested custom tags are planned for 2.4, the same as this ezcontent.

I don't see a point in spending time on limiting this and later enable all buttons as it's all planned for 2.4 and will not be released earlier.

@bdunogier
Copy link
Member

I knew you would say that, and it of course makes sense. As long as we don't end up skipping it in 2.4 for some reason, it will have to do. Thank you.

@dew326
Copy link
Member Author

dew326 commented Nov 8, 2018

I think we are safe in this case, @alongosz already has the nested custom tags in the current sprint.

@bdunogier
Copy link
Member

Okay, that sounds good. Thank you for the infos, @dew326.

@dew326 dew326 force-pushed the editable-ezcontent branch from c860432 to e04b411 Compare December 5, 2018 11:29
@micszo micszo self-assigned this Dec 6, 2018
Copy link
Member

@micszo micszo left a comment

Choose a reason for hiding this comment

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

Encountered issues occur on master also, will report separately.

@lserwatka lserwatka merged commit f0569d2 into ezsystems:master Dec 10, 2018
@micszo micszo removed their assignment Dec 10, 2018
@dew326 dew326 deleted the editable-ezcontent branch December 10, 2018 13:28
ViniTou pushed a commit that referenced this pull request May 12, 2023
Merge branch '2.3' of ezsystems/ezplatform-admin-ui into 4.3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

6 participants