LPS-130835 Create Balloon Editor Plugins #1070

julien · 2021-05-18T10:58:42Z

This pull request provides the Balloon Editor React component,
as well as the required CKEditor plugins.

Here's the list of tasks that have been worked on:

Instead of using the patch workflow available in liferay-ckeditor, a ballooneditor plugin has been created to override the default behavior of the balloonpanel and balloontoolbar plugins: but the end result is the same. Following the advice received here , all plugins have been create directly in DXP (in the frontend-editor-ckeditor-web module).

Screenshots

Selecting text from top to bottom

Selecting text from bottom to top

Selecting images

Adding alternative text to an image

Adding a link to an image

Adding an image from the document library

The plus button and it's toolbar (which allows adding images, videos, tables, and horizontal rules)

Adding a table from the plus button

Selecting a video

What's missing

Test plan

~~You'll need a local clone of the liferay-ckeditor repository and these changes~~ Not anymore, thanks to @markocikos who released a new version of liferay-ckeditor .
Fetch this branch
Deploy the frontend-editor-ckeditor-web module
Deploy the frontend-editor-ckeditor-sample-web module

Misc

Naming is hard: I tried to come up with names that make sense for plugins, but there's always room for improvement

You'll probably notice that I've added some eslint and prettier specific comments in some plugins (here's an example), this will need more investigation, but using the code below (as formatted by prettier) will not work as expected.

CKEDITOR.ui.balloonToolbarButton = CKEDITOR.tools.createClass({
    $(definition) {
         // ...
    }
});
// Will throw an error, about a non-existing constructor if you use:
new CKEDITOR.ui.balloonToolbarButton(...);

liferay-continuous-integration · 2021-05-18T10:58:47Z

CI is automatically triggering the following test suites:

ci:test:relevant
ci:test:sf

liferay-continuous-integration · 2021-05-18T11:03:07Z

✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 2525abbf42901829bc1f03a70f5fd85f7c4d0bd8

Sender Branch:

Branch Name: LPS-130835
Branch GIT ID: a6c47cf58ffce628cbb966f00f9ad00dc2ec5312

1 out of 1jobs PASSED

1 Successful Jobs:

test-portal-source-format

For more details click here.

liferay-continuous-integration · 2021-05-18T11:05:08Z

Jenkins Build:test-portal-source-format#5868

Jenkins Report:jenkins-report.html

Jenkins Suite:sf

Pull Request:liferay-frontend#1070

Testray Routine:EE Pull Request

Testray Build:[master] ci:test:sf - julien > liferay-frontend - PR#1070 - 2021-05-18[03:59:21]

Testray Importer:publish-testray-report#588

liferay-continuous-integration · 2021-05-18T13:19:47Z

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 23 out of 23 jobs passed in 1 hour 51 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 2525abbf42901829bc1f03a70f5fd85f7c4d0bd8

Upstream Comparison:

Branch GIT ID: 2525abbf42901829bc1f03a70f5fd85f7c4d0bd8
Jenkins Build URL: Acceptance Upstream DXP (master) #1875

ci:test:stable - 9 out of 9 jobs PASSED

9 Successful Jobs:

ci:test:relevant - 23 out of 23 jobs PASSED

23 Successful Jobs:

For more details click here.

Test bundle downloads:

liferay-portal-bundle-tomcat.tar.gz (mirrors: dlc, lax)

liferay-continuous-integration · 2021-05-18T13:24:14Z

Jenkins Build:test-portal-acceptance-pullrequest(master)#7771

Jenkins Report:jenkins-report.html

Jenkins Suite:relevant

Pull Request:liferay-frontend#1070

Testray Routine:EE Pull Request

Testray Build:[master] ci:test:relevant - julien > liferay-frontend - PR#1070 - 2021-05-18[04:36:07]

Testray Importer:publish-testray-report#592

diegonvs · 2021-06-02T22:30:19Z

...or-ckeditor-web/src/main/resources/META-INF/resources/_diffs/plugins/ballooneditor/plugin.js

+						}
+
+						if (y < editableClientRect.y) {
+							y = clientRect.bottom + padding;


From: https://liferay.slack.com/archives/C3JBR21HA/p1622644861109600?thread_ts=1622562932.067900&cid=C3JBR21HA

It solves:

Errors when positioning an image point 2

Misc points 1,2 and 3

julien · 2021-06-03T07:45:48Z

Hey @diegonvs, please be explicit about the error even when including a link to a Slack thread (Misc points 1,2 and 3 is pretty vague even when viewing the thread) - I'll have a look at the suggested solution, but I don't understand how this could affect "pasting text" and the "image drag handle".

We should discuss this, because I'm not seeing the same issues you mention

If I select a very large text and copy and just paste it throws some errors

In this .gif, I'm pasting 15 paragraphs of "Lorem Ipsum" a couple of times (note that the issues are related to cookies and not the editor, no error is thrown)

If I select a very large text and try to format something it throws some errors

In this .gif, I select 15 paragraphs of text and formatting it to bold, underline, italic and the removing the formatting: no errors are thrown

Looks like the image dragging handle is not working

In this .gif, I'm resizing the image a few times using the "handle", as I said in the thread "it's not pixel perfect", but it's not so bad either. This plugin comes with CKEditor, we haven't done any modifications so far.

I'm not trying to say that there are no errors, I'm not seeing what you described so if I didn't understand what you wrote, please include a .gif and the error itself. Thanks

Since you mention "dragging an image around in the editor", here's another .gif of we doing just that

Just to make things clear, even if this works, I'm assuming it could crash - this isn't a word processor, it's just a <div> element with the contenteditable attribute set to true and some "hacks" to make it work like a text editor, it's not perfect

To avoid this being "committed" since I don't agree, I'm deleting your suggestion

julien · 2021-06-03T10:13:05Z

When checking the bugs described by @diegonvs, I did see something I wanted to fix, which can be seen in the following screenshot

(I'm sure @dsanz will like this image 😄 )

I pushed another commit (0f6bf7d) and it fixes the issue. After thinking of various ways to do it via JS, I finally went for the straightforward approach using CSS, because it's much more simple and that's what CSS is for after all.

Here's a (slow) .gif of the result

julien · 2021-06-03T11:05:04Z

In 7a00079, I've added the long-awaited fix for the BGColor plugin which allows the text color to be changed.

Before

After

[EDIT] The message you'll see in the second .gif has not been added, it was temporary

julien · 2021-06-03T19:36:10Z

...or-ckeditor-web/src/main/resources/META-INF/resources/_diffs/plugins/ballooneditor/plugin.js

+
+				// Fix for BGColor button
+
+				var originalShowBlockFn = CKEDITOR.ui.panel.prototype.showBlock;


Not sure I like this Fn suffix.

diegonvs · 2021-06-03T21:19:06Z

I'll have a look at the suggested solution, but I don't understand how this could affect "pasting text" and the "image drag handle".

I was having trouble and some errors were thrown in console when executing these actions previously and I decided to investigate more about it. When debugging on console I notice a undefined value for clientRect in some cases and when seeing the scope of clientRect it was declared inside a if statement and for some cases it will not work and it will probably used to fix something but they forgot to use the current getBoundingClientRect variable which is editableClientRect and then I deployed and it worked well :)

When testing in other cases it worked. However, testing again it wasn't reproducible anymore but I found another issue and I think it doesn't have clear steps like:

When playing around:

Screen.Recording.2021-06-03.at.18.11.41.mov

Again trying to find the clear steps:

Screen.Recording.2021-06-03.at.18.18.30.mov

I wasn't able to repro anymore. :///

diegonvs · 2021-06-03T21:20:32Z

I was having trouble and some errors were thrown in console when executing these actions previously and I decided to investigate more about it. When debugging on console I notice a undefined value for clientRect in some cases and when seeing the scope of clientRect it was declared inside a if statement and for some cases it will not work and it will probably used to fix something but they forgot to use the current getBoundingClientRect variable which is editableClientRect and then I deployed and it worked well :)

Probably these errors happened to me as a shown in last GIF :/ It is something intermittently(I don't like this word)

julien · 2021-06-04T06:56:38Z

@diegonvs looking at the videos you added it makes sense: the styles plugin (the one that you're clicking on in the video, and richCombo plugin) doesn't work when used in the balloontoolbar, this was "fixed" in 1e3f848 so you might have been testing with something "cached", in any case this should now work as expected - thanks.

...r-ckeditor-web/src/main/resources/META-INF/resources/_diffs/plugins/toolbarbuttons/plugin.js

javiergamarra · 2021-07-04T23:35:31Z

This: #1150 (comment) I think it only works the second time (because the focusable is registered in the onchange callback, it has to be executed once). I would register a global one selecting the whole plugin as I did in my example. There is a callback when the plugin has been loaded that could be useful.

markocikos · 2021-07-05T13:13:23Z

About ES6, #1150 (comment)

@diegonvs I believe we can use template literals, and all other ES6+ goodies. I used it in addimages plugin. I don't think there is anything special about that plugin 🤔

Looking at #1150 (comment), @julien, were you maybe thinking about a plugin in ckeditor repo? On that repo we don't have babel integration, so something like liferay/liferay-ckeditor#114 makes sense to be in ES5.

julien · 2021-07-05T13:22:13Z

@markocikos yes exactly: I meant to say that we can use ES6 in dxp but not if the plugin is in CKEditor directly.
Still it feels a bit strange to have plugins that are in ES5 and other in ES6, but I guess it's just a matter of knowing when we can or can't use ES6.

diegonvs · 2021-07-05T17:26:48Z

@julien, were you maybe thinking about a plugin in ckeditor repo? On that repo we don't have babel integration, so something like liferay/liferay-ckeditor#114 makes sense to be in ES5.
I believe we can use template literals, and all other ES6+ goodies.

It will look good then. I was understanding this plugin(on frontend-editor-ckeditor-web will be processed somewhere that don't support ES6 but the babel reason thing looks fine.

Still it feels a bit strange to have plugins that are in ES5 and other in ES6, but I guess it's just a matter of knowing when we can or can't use ES6.

Yes. Is there a place to write about it and prevent people to just use ES5 there?

liferay-continuous-integration · 2021-07-14T12:04:00Z

✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 923fcc1a6b873674c559758a01ff7a91b7486c27

Sender Branch:

Branch Name: LPS-130835
Branch GIT ID: 2a34df2872a5ea87e3b2a42bb5c5c5c40337836e

1 out of 1jobs PASSED

1 Successful Jobs:

test-portal-source-format

For more details click here.

liferay-continuous-integration · 2021-07-14T14:13:54Z

✔️ ci:test:stable - 9 out of 9 jobs passed

❌ ci:test:relevant - 20 out of 22 jobs passed in 2 hours 12 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 923fcc1a6b873674c559758a01ff7a91b7486c27

Upstream Comparison:

Branch GIT ID: 923fcc1a6b873674c559758a01ff7a91b7486c27
Jenkins Build URL: Acceptance Upstream DXP (master) #2103

ci:test:stable - 9 out of 9 jobs PASSED

9 Successful Jobs:

ci:test:relevant - 20 out of 22 jobs PASSED

2 Failed Jobs:

20 Successful Jobs:

For more details click here.

Failures unique to this pull:

test-portal-acceptance-pullrequest-batch(master)/semantic-versioning-jdk8/0

Job Results:

0 Tests Passed.
1 Test Failed.

AXIS_VARIABLE=0,label_exp=!master #231782

clean-up-java-processes:
   [delete] Deleting: /opt/dev/projects/github/liferay-portal/clean_up_java_processes.sh
[stopwatch] [run.batch.test.setup: 33:32.924 sec]
     [echo] run.batch.test.action.start.timestamp: 07-14-2021 07:05:35:937 PDT
     [echo] Checking for baseline log files.
     [echo] 
     [echo] ##
     [echo] ## /opt/dev/projects/github/liferay-portal/baseline-reports/portal-impl-7.log
     [echo] ##
     [echo] 
[beanshell] [Baseline Warning] Bundle Version Change Recommended: 7.8.1
[beanshell] 
     [echo] 
     [echo] ##
     [echo] ## /opt/dev/projects/github/liferay-portal/baseline-reports/portal-test-9.log
     [echo] ##
     [echo] 
[beanshell] [Baseline Warning] Bundle Version Change Recommended: 9.3.5
[beanshell] 
     [echo] 
     [echo] ##
     [echo] ## /opt/dev/projects/github/liferay-portal/baseline-reports/util-bridges-8.log
     [echo] ##
     [echo] 
[beanshell] [Baseline Warning] Bundle Version Change Recommended: 8.1.1
[beanshell] 
     [echo] 
     [echo] ##
     [echo] ## /opt/dev/projects/github/liferay-portal/baseline-reports/util-java-6.log
     [echo] ##
     [echo] 
[beanshell] [Baseline Warning] Bundle Version Change Recommended: 6.0.4
[beanshell] 
     [echo] 
     [echo] ##
     [echo] ## /opt/dev/projects/github/liferay-portal/baseline-reports/util-slf4j-6.log
     [echo] ##
     [echo] 
[beanshell] [Baseline Warning] Bundle Version Change Recommended: 6.0.4
[beanshell] 
[stopwatch] [run.batch.test.action: 15.645 sec]
     [echo] Semantic versioning is incorrect.
      [get] Getting: http://test-1-18/job/test-portal-acceptance-pullrequest-batch(master)/AXIS_VARIABLE=0,label_exp=!master/231782//consoleText
      [get] To: /opt/dev/projects/github/liferay-portal/20210714070551582.txt
   [delete] Deleting: /opt/dev/projects/github/liferay-portal/20210714070551582.txt
   [delete] Deleting: /opt/dev/projects/github/liferay-portal/null1564969090.properties

For upstream results, click here.

Test bundle downloads:

liferay-portal-bundle-tomcat.tar.gz (mirrors: dlc, lax)

diegonvs · 2021-07-14T18:48:19Z

ci:forward

liferay-continuous-integration · 2021-07-14T18:48:24Z

CI is automatically triggering the following test suites:

ci:test:relevant
ci:test:sf

The pull request will automatically be forwarded to the user brianchandotcom if the following test suites pass:

ci:test:relevant
ci:test:sf
ci:test:stable

liferay-continuous-integration · 2021-07-14T18:48:27Z

Skipping previously passed test suites:
ci:test:sf

diegonvs · 2021-07-14T18:50:28Z

Here We go again

In brazilian portuguese 😂

liferay-continuous-integration · 2021-07-14T20:59:33Z

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 22 out of 22 jobs passed in 2 hours 10 minutes

Click here for more details.

Base Branch:

Branch Name: master
Branch GIT ID: 0b91010c2ef77d4c61a3b74503d0220f88be8f04

Upstream Comparison:

Branch GIT ID: 923fcc1a6b873674c559758a01ff7a91b7486c27
Jenkins Build URL: Acceptance Upstream DXP (master) #2103

ci:test:stable - 9 out of 9 jobs PASSED

9 Successful Jobs:

ci:test:relevant - 22 out of 22 jobs PASSED

22 Successful Jobs:

For more details click here.

Test bundle downloads:

liferay-portal-bundle-tomcat.tar.gz (mirrors: dlc, lax)

liferay-continuous-integration · 2021-07-14T21:00:04Z

All required test suite(s) passed.
Forwarding pull request to brianchandotcom.
Console

liferay-continuous-integration · 2021-07-14T21:00:20Z

Pull request has been successfully forwarded to brianchandotcom#104367
Console

liferay-continuous-integration · 2021-07-14T21:03:27Z

Jenkins Build:test-portal-acceptance-pullrequest(master)#254

Jenkins Report:jenkins-report.html

Jenkins Suite:relevant

Pull Request:liferay-frontend#1070

Testray Routine:EE Pull Request

Testray Build:[master] ci:test:relevant - julien > liferay-frontend - PR#1070 - 2021-07-14[11:48:33]

Testray Importer:publish-testray-report#48

liferay-continuous-integration added ci:test:sf - pending ci:test:sf - success and removed ci:test:sf - pending labels May 18, 2021

liferay-continuous-integration added ci:test:relevant - pending ci:test:stable - pending ci:test:relevant - success ci:test:stable - success and removed ci:test:relevant - pending ci:test:stable - pending labels May 18, 2021

javiergamarra added the s-dxp One of the DXP squads should review this pull request (deprecated) label May 18, 2021

diegonvs reviewed Jun 2, 2021

View reviewed changes

Repository owner deleted a comment from diegonvs Jun 3, 2021

julien commented Jun 3, 2021

View reviewed changes

diegonvs mentioned this pull request Jun 12, 2021

LPS-130638 Spacing/Line Height Button of Balloon Toolbar #1150

Closed

diegonvs reviewed Jun 30, 2021

View reviewed changes

...r-ckeditor-web/src/main/resources/META-INF/resources/_diffs/plugins/toolbarbuttons/plugin.js Outdated Show resolved Hide resolved

liferay-continuous-integration added ci:test:relevant - pending ci:test:stable - pending ci:test:relevant - failure ci:test:stable - success and removed ci:test:relevant - success ci:test:stable - success ci:test:relevant - pending ci:test:stable - pending labels Jul 14, 2021

diegonvs mentioned this pull request Jul 14, 2021

LPS-133018 frontend-js-tabs-support-web Tab content and trigger no lo… #1247

Closed

liferay-continuous-integration added ci:test:relevant - pending ci:test:stable - pending ci:test:relevant - success ci:test:stable - success and removed ci:test:relevant - failure ci:test:stable - success ci:test:relevant - pending ci:test:stable - pending labels Jul 14, 2021

liferay-continuous-integration mentioned this pull request Jul 14, 2021

LPS-130835 Create Balloon Editor Plugins brianchandotcom/liferay-portal#104367

Closed

17 tasks

liferay-continuous-integration closed this Jul 14, 2021

julien deleted the LPS-130835 branch September 3, 2021 06:03


		// Fix for BGColor button

		var originalShowBlockFn = CKEDITOR.ui.panel.prototype.showBlock;

LPS-130835 Create Balloon Editor Plugins #1070

LPS-130835 Create Balloon Editor Plugins #1070

Conversation

julien commented May 18, 2021 • edited Loading

liferay-continuous-integration commented May 18, 2021

liferay-continuous-integration commented May 18, 2021

✔️ ci:test:sf - 1 out of 1 jobs passed in 4 minutes

Base Branch:

Sender Branch:

For more details click here.

liferay-continuous-integration commented May 18, 2021

liferay-continuous-integration commented May 18, 2021

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 23 out of 23 jobs passed in 1 hour 51 minutes

Base Branch:

Upstream Comparison:

For more details click here.

Test bundle downloads:

liferay-continuous-integration commented May 18, 2021

diegonvs Jun 2, 2021

Choose a reason for hiding this comment

julien commented Jun 3, 2021

julien commented Jun 3, 2021 • edited Loading

julien commented Jun 3, 2021 • edited Loading

julien Jun 3, 2021

Choose a reason for hiding this comment

diegonvs commented Jun 3, 2021

diegonvs commented Jun 3, 2021 • edited Loading

julien commented Jun 4, 2021

javiergamarra commented Jul 4, 2021

markocikos commented Jul 5, 2021 • edited Loading

julien commented Jul 5, 2021

diegonvs commented Jul 5, 2021

liferay-continuous-integration commented Jul 14, 2021

✔️ ci:test:sf - 1 out of 1 jobs passed in 3 minutes

Base Branch:

Sender Branch:

For more details click here.

liferay-continuous-integration commented Jul 14, 2021

✔️ ci:test:stable - 9 out of 9 jobs passed

❌ ci:test:relevant - 20 out of 22 jobs passed in 2 hours 12 minutes

Base Branch:

Upstream Comparison:

2 Failed Jobs:

For more details click here.

Failures unique to this pull:

test-portal-acceptance-pullrequest-batch(master)/semantic-versioning-jdk8/0

Job Results:

For upstream results, click here.

Test bundle downloads:

diegonvs commented Jul 14, 2021

liferay-continuous-integration commented Jul 14, 2021

liferay-continuous-integration commented Jul 14, 2021

diegonvs commented Jul 14, 2021 • edited Loading

liferay-continuous-integration commented Jul 14, 2021

✔️ ci:test:stable - 9 out of 9 jobs passed

✔️ ci:test:relevant - 22 out of 22 jobs passed in 2 hours 10 minutes

Base Branch:

Upstream Comparison:

For more details click here.

Test bundle downloads:

liferay-continuous-integration commented Jul 14, 2021

liferay-continuous-integration commented Jul 14, 2021

liferay-continuous-integration commented Jul 14, 2021

julien commented May 18, 2021 •

edited

Loading

julien commented Jun 3, 2021 •

edited

Loading

julien commented Jun 3, 2021 •

edited

Loading

diegonvs commented Jun 3, 2021 •

edited

Loading

markocikos commented Jul 5, 2021 •

edited

Loading

diegonvs commented Jul 14, 2021 •

edited

Loading