fix: remove padding and background-color in page editor .cke_editable

[mateomustapic]

This is a bug fix for https://issues.liferay.com/browse/LPS-117468
Reseting border and background styles for cke_editable css class in page editor.

The issue with overlay that is preventing unrelated UI interactions on the page should be fixed in #98

Steps to reproduce

1. Edit Hello World page (or create a content page with a heading fragment)
2. Double click on a text editable to start editing.

Also, see below a video

[mateomustapic]

[markocikos]

Styles specific to page editor should be in the page editor module, not in this repo.

[carloslancha]

Exactly, the idea is try to fix this as part of the general skin. If we can't because it's not a bug but an specific behavior that differs from what we want to provide with the skin, then yes, this should be in portal. That's what we need to analyze.

[markocikos]

@mateomustapic It is a bit odd to me that the product team would like a different background and border than standard. The alloyeditor in the page editor seems to be its standard use case. I suspect that this might be just a misleading ticket description, where the actual issue is bugged focus and selection behavior, caused by the issue in #98. I might be wrong on this one, but we should definitely check this with product team before forwarding.

[mateomustapic]

@mateomustapic It is a bit odd to me that the product team would like a different background and border than standard. The alloyeditor in the page editor seems to be its standard use case. I suspect that this might be just a misleading ticket description, where the actual issue is bugged focus and selection behavior, caused by the issue in #98. I might be wrong on this one, but we should definitely check this with product team before forwarding.

I agree that it makes sense to check with product team regarding a background colour (although I think this is required due to background of Hello World app and other pages), still I think the padding is off for all inline editors and it should be set to 0 in the skin.

[markocikos]

I think the padding is off for all inline editors and it should be set to 0 in the skin.

To me it looks worse if you remove padding.

With current padding

With padding: 0

I'm not a designer, so this is only my opinion. But before doing this, I would strongly suggest checking with Lexicon team.

[carloslancha]

Hey guys, could you please have a conversation with @p2kmgcl about this? He's from Echo Team and the reporter of the issue.

Each application has (relative) freedom to customize they're styling so if they want a different behavior is up to them.

We must difference between:

• What's actually a bug, if there's any
• What's something that needs to be adjusted in the application styling

Anyway, @p2kmgcl would you mind to add a more detailed description to the ticket with examples of how it should look and what is exactly not looking as you expect?

[p2kmgcl]

Hey guys,

This bug is only about Page Editor, not Web Content editing or anything else. As described in the ticket, it did have no border, background and padding, bug this changed last week. I opened this as a bug because we didn't need to add any extra CSS to customize, but I am not sure if we where specifying some configuration that made it look like that. This is the configuration that we are using:

{
  "editorConfig": {
    "filebrowserBrowseUrl": "http://localhost:8080/group/guest/~/control_panel/manage/-/select/file%2Clayout/selectItem?_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_0_json=%7B%22desiredItemSelectorReturnTypes%22%3A%22com.liferay.item.selector.criteria.URLItemSelectorReturnType%22%7D&_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_1_json=%7B%22checkDisplayPage%22%3Afalse%2C%22desiredItemSelectorReturnTypes%22%3A%22com.liferay.item.selector.criteria.URLItemSelectorReturnType%22%2C%22enableCurrentPage%22%3Afalse%2C%22followURLOnTitleClick%22%3Afalse%2C%22multiSelection%22%3Afalse%2C%22showActionsMenu%22%3Afalse%2C%22showHiddenPages%22%3Atrue%2C%22showPrivatePages%22%3Atrue%2C%22showPublicPages%22%3Atrue%7D&p_p_auth=CthcsxHW",
    "enterMode": 2,
    "removePlugins": "contextmenu,elementspath,floatingspace,image,link,liststyle,magicline,resize,tabletools,toolbar,ae_embed",
    "allowedContent": "",
    "extraPlugins": "ae_autolink,ae_dragresize,ae_addimages,ae_imagealignment,ae_placeholder,ae_selectionregion,ae_tableresize,ae_tabletools,ae_uicore,itemselector,media,adaptivemedia",
    "filebrowserImageBrowseUrl": "http://localhost:8080/group/guest/~/control_panel/manage/-/select/image%2Curl/_EDITOR_NAME_selectImage?_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_0_json=%7B%22desiredItemSelectorReturnTypes%22%3A%22downloadurl%22%7D&_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_1_json=%7B%22desiredItemSelectorReturnTypes%22%3A%22com.liferay.item.selector.criteria.URLItemSelectorReturnType%22%7D&p_p_auth=CthcsxHW",
    "disallowedContent": "br",
    "filebrowserImageBrowseLinkUrl": "http://localhost:8080/group/guest/~/control_panel/manage/-/select/image%2Curl/_EDITOR_NAME_selectImage?_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_0_json=%7B%22desiredItemSelectorReturnTypes%22%3A%22downloadurl%22%7D&_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_1_json=%7B%22desiredItemSelectorReturnTypes%22%3A%22com.liferay.item.selector.criteria.URLItemSelectorReturnType%22%7D&p_p_auth=CthcsxHW",
    "toolbars": {},
    "documentBrowseLinkUrl": "http://localhost:8080/group/guest/~/control_panel/manage/-/select/file%2Clayout/_EDITOR_NAME_selectItem?_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_0_json=%7B%22desiredItemSelectorReturnTypes%22%3A%22downloadurl%22%7D&_com_liferay_item_selector_web_portlet_ItemSelectorPortlet_1_json=%7B%22checkDisplayPage%22%3Afalse%2C%22desiredItemSelectorReturnTypes%22%3A%22com.liferay.item.selector.criteria.URLItemSelectorReturnType%22%2C%22enableCurrentPage%22%3Afalse%2C%22followURLOnTitleClick%22%3Afalse%2C%22multiSelection%22%3Afalse%2C%22showActionsMenu%22%3Afalse%2C%22showHiddenPages%22%3Atrue%2C%22showPrivatePages%22%3Atrue%2C%22showPublicPages%22%3Atrue%7D&p_p_auth=CthcsxHW"
  },
  "editorOptions": {
    "dynamicAttributes": {},
    "textMode": false,
    "uploadItemReturnType": null,
    "uploadURL": null
  }
}

I am going to update the JIRA ticket with some screenshots.

[p2kmgcl]

This is a screenshot of the same page in GA4:

[mateomustapic]

hey @p2kmgcl thanks for this info.
I think that this has nothing to do with the configuration.
The only things I see are off, are padding and background-color which appeared with skin update last week.
As you can see in my screenshot above, the behaviour is now as you describe it.
If this is true, changes in this PR would fix this, the only change which should be done is to add these modifications in Page Editor stylesheet FragmentContent.scss (since these changes are required only in ) here =>
https://github.com/liferay-frontend/liferay-portal/blob/master/modules/apps/layout/layout-content-page-editor-web/src/main/resources/META-INF/resources/page_editor/app/components/fragment-content/FragmentContent.scss

[carloslancha]

So, @mateomustapic @p2kmgcl do we agree that this is happening because page editor has a different behavior/styling than the rest of alloy editor instances in portal?

If so, let's close this one and just add changes directly into Page Editor. @p2kmgcl ?

[p2kmgcl]

That's ok for me

[jbalsas]

Hey @carloslancha, @mateomustapic, not sure how the new skin is causing this since we're still using the old skin for AlloyEditor?

Maybe we removed too much CSS when migrating the skin and should be placed back?

It's one thing that CKEditor looks different, but AlloyEditor should've stayed the same in the process.

There could be other usages of AlloyEditor in other places that should also remain unchanged, so I'd suggest investigating what changed and put it back, since this can't be a PageEditor only issue (even though we mostly only use it there)

[carloslancha]

@jbalsas probably because we introduced a background-color for the content in the new skin that previously was unset or transparent. If Alloy is not specifing that it will inherit Ckeditor one, so it's not something we can't avoid breaking and will need to be replaced in portal (or in alloy editor's skin)

[jbalsas]

But AlloyEditor was using moono-lisa before and it's still using it. So the fact that we changed moono-lexicon should not affect AlloyEditor. The only way I see this could affect it is if we removed code from master when removing the .ae-editable or .alloyeditor custom css

[carloslancha]

AFAIK AlloyEditor uses whatever the version of CKEditor is in portal, and in portal we have a version with moono-lexicon... At least that's what I found out with @julien some weeks ago 🤷

[jbalsas]

AFAIK AlloyEditor uses whatever the version of CKEditor is in portal, and in portal we have a version with moono-lexicon... At least that's what I found out with @julien some weeks ago 🤷

Keep digging in your memory, I know this is somewhere around there: LPS-116201 Use moono-lisa skin in frontend-editor-alloyeditor-web because you thanked @julien for it! :)