-
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
LPD-33712 Add css to combopanel when maximized #211
Conversation
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.
@fortunatomaldonado please see comment inline
+ if (combopanel && maximizedEditor) { | ||
+ combopanel.classList.add('cke_maximized'); | ||
+ } else { | ||
+ combopanel.classList.remove('cke_maximized'); | ||
+ } |
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.
Can we achieve this just with CSS around here? I'd like to avoid doing a patch, if we don't need to. In addition, this change makes me nervous, as maximized plugin might rely on cke_maximized
for variety of unrelated features.
Maybe something like two different set of styles, based on combination of classes? You may use override by more specific style, or :not()
if that's not working.
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.
Hello @markocikos
I tried doing just that but unfortunately, only the editor element gets cke_maximized
so there is no telling the stylescombo panel when the editor is maximized since the stylescombo panel element is out of scope of the editor element.
We could try adding an eventListenser for the stylecombo button in Liferay so it check if the editor is maximized and then add the css. So essentially this fix but in Liferay.
Let me know what you think.
Thank you!
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.
@fortunatomaldonado After looking more thoroughly into this, yeah, you are right, this cannot be fixed just with CSS. Adding a listener like you suggested could work, but it would imply moving / forking plugin to DXP, an overkill for this bugfix. We could also dynamically modify z-index
es of toolbars, but that may have regressions elsewhere. Overall, in this case, patch like in this PR looks like the best way to go. I'd only make one or two minor changes to solution as it is:
- We should make the new class on stylescombo something other than
cke_maximized
, so it doesn't accidentally trigger some style or code in the maximize plugin. Maybelfr-maximized
? - Is there a native reference to set class with, something like
this.panel.className
? If it's hard to find, no problem, it's not a big deal if we keep the query selector.
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.
- That makes sense, I changed the class name to
lfr-maximized
- I wasn't able to find any reference of the panel just the contents of it. I left it as is.
Thank you for the help and let me know if there is anything else needed to change.
This commit autogenerated with "./ck.sh".
b2ee2c6
to
62fe441
Compare
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.
@fortunatomaldonado LGTM! ✨
https://liferay.atlassian.net/browse/LPP-55194
https://liferay.atlassian.net/browse/LPD-33712
This fixes an issue where the stylescombo panel is not visible when ckeditor is maximized.
This is due to the z-index being lower than the maximized editor which has a z-index of 9995.
I could not soley increase the z-index of the panel since it would cause a regression here: https://liferay.atlassian.net/browse/LPS-158973
I decided to add the css to the panel so that when clicked and the editor is maximized, it will increase the z-index to 10000. When the editor is no longer maximized, it will remove the css.
We also need the fix here in order to get the full fix: https://github.com/fortunatomaldonado/liferay-portal/commits/LPD-33712/
If this passes, I will add the ckeditor version change to the above branch.
Please let me know if there are any questions or comments about this.