search

liferay / liferay-ckeditor

Other
8 stars 49 forks source link

fix: LPS-130834 CKEditor is not displaying the full source code text on 7.0.x #165

Closed georgel-pop-lr closed 3 years ago

georgel-pop-lr commented 3 years ago

This PR fixes https://github.com/liferay/liferay-ckeditor/issues/163

wincent commented 3 years ago

@georgel-pop-lr: Are you able to include a "Test plan" here? Basically, want to see what steps you took to verify that this fixes the issue on 7.0, and doesn't regress anything on later branches.

julien commented 3 years ago

@georgel-pop-lr just so you know; I tested this on master, following the test plan you left in the original issue (create a web content article with lots of text and switched to source mode) and I didn't see anything strange.

georgel-pop-lr commented 3 years ago

@georgel-pop-lr just so you know; I tested this on master, following the test plan you left in the original issue (create a web content article with lots of text and switched to source mode) and I didn't see anything strange.

Hi @julien,

It is true as this seems to affect only 7.0.x and would not happen to master where we have a scroll outside the editor and that's why this fix should not affect other branches.

Please check on 7.0.x

Regards.

georgel-pop-lr commented 3 years ago

@georgel-pop-lr: Are you able to include a "Test plan" here? Basically, want to see what steps you took to verify that this fixes the issue on 7.0, and doesn't regress anything on later branches.

Hi @wincent,

I already had a call with @julien and explained why we need these changes, basically we need to synchronize the height of code mirror with the height of the editor.

I changed the commit so is including the initialization of the height from config when we first open source code.

With these changes I confirm that everything works as expected.

Tomorrow I will check if any branches would be impacted by these, but it shouldn't.

Regards.

georgel-pop-lr commented 3 years ago

@georgel-pop-lr: Are you able to include a "Test plan" here? Basically, want to see what steps you took to verify that this fixes the issue on 7.0, and doesn't regress anything on later branches.

Hi @wincent,

I already had a call with @julien and explained why we need these changes, basically we need to synchronize the height of code mirror with the height of the editor.

I changed the commit so is including the initialization of the height from config when we first open source code.

With these changes I confirm that everything works as expected.

Tomorrow I will check if any branches would be impacted by these, but it shouldn't.

Regards.

Hi @wincent @julien,

I check on each branch and made some changes, we need to get the parent height