-
Notifications
You must be signed in to change notification settings - Fork 49
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
fix: remove css classes as well when changing style #156
Conversation
- range.enlarge( CKEDITOR.ENLARGE_INLINE ); | ||
+ range.enlarge( CKEDITOR.ENLARGE_ELEMENT ); |
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.
I don't really understand how this fixes the bug, but perhaps @julien does. Are you able to break it down for us @NemethNorbert?
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.
Hey @wincent,
range represents a delimited piece of content in a DOM Document. It is contiguous in the sense that it can be characterized as selecting all of the content between a pair of boundary-points.
range.enlarge() Expands the range so that partial units are completely contained. It relieves two parameters:
The unit type to expand with. Use one of following values: CKEDITOR.ENLARGE_BLOCK_CONTENTS, CKEDITOR.ENLARGE_ELEMENT, CKEDITOR.ENLARGE_INLINE, CKEDITOR.ENLARGE_LIST_ITEM_CONTENTS.
and
[ excludeBrs ] : Boolean
Whether include line-breaks when expanding.
As far as I understood the documentation, by using CKEDITOR.ENLARGE_ELEMENT, I basically expand the selection to the whole element, that will also include the class attribute of the html element.
resources:
https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR_dom_range.html#method-enlarge
https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR.html#property-ENLARGE_ELEMENT
https://ckeditor.com/docs/ckeditor4/latest/api/CKEDITOR.html#property-ENLARGE_INLINE
Thanks
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.
Wonderful explanation. Thanks @NemethNorbert! Next time consider including it in your commit message (which is used to seed the PR description, so we'd see it there too).
I think we can 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.
Cut v4.14.1-liferay.19 with this @NemethNorbert. Will shortly send a PR updating |
Thank you @wincent , please use LPS-127732 to update the version on master. I will take care of the backports. Thanks, |
CI running now over at: liferay-frontend/liferay-portal#895 |
Hey,
LPS-127732 (we can use this to update the ckeditor version in Liferay),
I modified the solution and generated a patch as suggested in #155
Please review it,
Thanks