ckeditor5-link - ckeditor in dialog box where main window is scrolled down - Position of link edit dialog

IgorWolbers commented 9 months ago

📝 Provide detailed reproduction steps (if any)

Have a main window that can scroll vertically
Open a dialog box that contains the ckeditor
When the main window scrolls down a little open the dialog and then create a new link
Notice the link dialog position is fixed to the top of the main window / body html

See also the attached gif that illustrates the problem.

✔️ Expected result

The link dialog should be fixed relative to the dialog window that contains the ckeditor

❌ Actual result

The link dialog is fixed relative to the html body so it scrolls out of screen as the main window is scrolled down

📃 Other details

Browser: Edge
OS: Windows
First affected CKEditor version: Latest - 40.1.0
Installed CKEditor plugins: @ckeditor/ckeditor5-link

If you'd like to see this fixed sooner, add a 👍 reaction to this post.

Reproduction using Stackblitz and Angular Material

Reproduction Project

GIF (if it does not auto-play please click on the image) Problem Illustrated

Witoso commented 9 months ago

It looks like the editor didn't blur correctly on closing the dialog. Would you mind verifying if blur is fired?

IgorWolbers commented 9 months ago

@Witoso thanks for your response.

I am not sure if that is related or not but I can duplicate the issue without first opening the editor with the window scroll position at the top. See the gif below.

Although angular is being used as the host the editor is being loaded as if angular is not used. The repo and code takes angular out of the picture entirely (I hope). I did that so that there was no chance of angular specific integration code being blamed. Everything else has been working perfectly, this is the only issue that we have encountered so far. See also the stackblitz for the reproduction code but I will include the relevant loading code below for convenience. It is nothing more than a button that opens an angular material dialog box which hosts just the ckeditor.

example-dialog.ts

ngAfterViewInit() {
  ClassicEditor.create(document.querySelector('#editor'), {
    toolbar: {
      items: ['heading', 'bold', 'italic', 'underline', 'link'],
    },
    language: 'en',
  }).catch((error: any) => {
    console.error(error);
  });
}

example-dialog.html

<h1>Editor</h1>
<div>
  <div id="editor"></div>
</div>

GIF Reproduction where window is opened immediately after window load and scroll

Link position with dialog and scroll Start at Top

Witoso commented 9 months ago

@oleq, thoughts?

oleq commented 9 months ago

Thank you @IgorWolbers for a detailed summary and the code. It was very helpful for me while debugging.

The issue is on CKEditor side and it comes down to the fact that the dialog system used in the demo adds position: fixed and a negative top value to the <html> element. I have to be honest with you, though, this is the first time I've ever seen (or noticed) such styling on <html> element. It's very creative but still quite unusual.

You can reproduce the issue on your own in any editor demo, such as https://ckeditor.com/docs/ckeditor5/latest/examples/builds/classic-editor.html by just adding

html {
    position: fixed;
    top: -30px;
    left: -30px;
}

The issue can also be addressed like this

diff --git a/packages/ckeditor5-utils/src/dom/rect.ts b/packages/ckeditor5-utils/src/dom/rect.ts
index 046fff1f9b..839d3f8153 100644
--- a/packages/ckeditor5-utils/src/dom/rect.ts
+++ b/packages/ckeditor5-utils/src/dom/rect.ts
@@ -385,6 +385,8 @@ export default class Rect {
                            }
                   }

+                  shiftRectToCompensatePositionedAncestor( absoluteRect, document.documentElement );
+
                   return absoluteRect;
          }

The fix will require some attention from the QA team first to make sure it does not break anything else.

IgorWolbers commented 9 months ago

Thank you @oleq! Should I submit a PR for the fix based on your last comment?

oleq commented 9 months ago

Thank you @oleq! Should I submit a PR for the fix based on your last comment?

Your contribution would be greatly appreciated not only in terms of the code but also in QA 🙂

IgorWolbers commented 9 months ago

Your contribution would be greatly appreciated not only in terms of the code but also in QA 🙂

@oleq - I have created a PR but am not allowed to sign anything (like a CLA). Seeing as you provided the fix suggestion and it is obviously not something I thought of I hope this is not an issue for getting the PR merged.

Thank you very much for your timely responses and all of the work you do on this repo.

Witoso commented 9 months ago

We will add it to our issues queue.

ckeditor / ckeditor5