Problem/Motivation

I used ckeditor 5 and admin toolbar, when I focused cursor to my ckeditor 5 AND scroll down, the ckeditor 5 goes over the admin bar briefly.

CKEditor overlap

Steps to reproduce

Ckeditor 5 (in my case, the ckeditor has two rows of buttons, a great height (493px , toolbar included).
Drupal 10.2.4
Admin toolbar 3.4.2

Proposed resolution

By increasing the z-index manually in my browser, I managed to fix the problem (I tested quickly with 1500 instead 501 in .toolbar .toolbar-tray and .toolbar-oriented .toolbar-bar

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Tichris59 created an issue. See original summary.

amit.mall’s picture

StatusFileSize
new484 bytes

I have added a patch for this, with variable z-index. Use the attched patch: 3426402.patch

amit.mall’s picture

Status: Active » Needs review
divya.sejekan’s picture

StatusFileSize
new1.73 MB

Im not able to reproduce this issue . Attaching issue recording
Using Drupal 10.2.5 , Admin tool bar - 3.4.2

Testing steps :
1. Install drupal and admin tool bar module
2. Edit the editor for showing tools in 2 line
3. Create a content verify the ckeditor

gg24’s picture

StatusFileSize
new315 bytes

I have created a new patch. The above patch didn't get applied for me.

Please review.

gg24’s picture

StatusFileSize
new313 bytes

Uploading a new working patch.

sdhruvi5142’s picture

StatusFileSize
new195.92 KB
new286.9 KB

Hi,
I've verified and tested Patch and applied "Patch #6" successfully on 10.2.x Version the changes are working as expected.

Testing Steps:
1. Install drupal and admin tool bar module
2. Edit the editor and select the "Full HTML" option in Text Format
3. Scroll up the page and Observe the changes there.

Testing Result
After scrolling up the page the there is no Z-index Issue between admin toolbar and ckeditor 5 and it is working as expected.

Attaching the screenshots for reference.

Hence moving to RTBC!
Thanks

sdhruvi5142’s picture

Status: Needs review » Reviewed & tested by the community
dydave’s picture

Version: 3.4.2 » 3.x-dev
Status: Reviewed & tested by the community » Active
StatusFileSize
new95.69 KB

Just tested this on:

  • D10.3.0
  • admin_toolbar:3.x-dev
  • Current Windows Chrome browser

 
and was able to reproduce, see screenshot below:


 

Thanks a lot @divya.sejekan for the video which helped better understanding how this should be tested:

You need to scroll all the way down to the bottom of the editor and there is a really small/minor overlap where the CKEditor 5 Toolbar displays over the admin_toolbar.
But really, from what I have tested, this seems like a very minor issue.

Otherwise, @sdhruvi5142:
Could you please maybe provide a bit more information on the context of the issue and how it could be reproduced ?
See for example this isse #3462559: Link to "Block" content overview page not displayed with a recorded GIF file in the issue summary.

Thanks in advance!

japerry made their first commit to this issue’s fork.

japerry’s picture

Status: Active » Needs work
StatusFileSize
new118.72 KB

I'm not convinced either one of these patches (#6 or #2) are what we want. While it moves down the ckeditor toolbar, it now hides the first item from the admin_toolbar dropdown. See attached screenshot.

neptune-dc’s picture

> I'm not convinced either one of these patches (#6 or #2) are what we want. While it moves down the ckeditor toolbar, it now hides the first item from the admin_toolbar dropdown. See attached screenshot.

You must not have implemented the patch correctly. Or your theme is adding additional zindexes to the module.

When I implement patch #6, it fully fixes the issue.

dydave’s picture

Thanks Ben (@neptune-dc) for the recent feedback on this issue!

I took a closer look a few weeks back and was actually able to trace this down to the initialization of the view port offset of the CKEditor5 Toolbar....

In other words:
With the latest stable, for example 3.6.1, if you try reproducing the problem below:

Then, try resizing the window (without refreshing, just a small change of the window size):
If you test again, you should see the toolbar displays at the expected position right underneath the Admin Toolbar.

Therefore, it somewhere boils down to the initialization of some JS settings variables and/or to a "resize" event triggered.

See potentially related issue #3526123: <thead> element and CKEditor toolbar hang, when Toolbar is hidden and Drupal.displace();

Maybe approaches with JS instead of CSS should be further explored.
It is likely this issue could be fixed with a few lines of JS code (?!)

Any testing, feedback and reviews would be greatly appreciated.
Thanks in advance!

ressa’s picture

Yes @dydave, I did also notice occasionally that the CKEditor toolbar would have the 80px offset with "Hidden" or Hidden, show on scroll up", which is being worked on in #3526123: <thead> element and CKEditor toolbar hang, when Toolbar is hidden, it could be all connected :)

When I tried earlier to recreate it reliably, I could not ... but I tried again just now, and succeeded, so I have added fixing CKEditor toolbar offset as a task in the other issue.

To successfully recreate it, you might need to have content in the field, as well as place the cursor in the field.

Perhaps it's most ideal to look at all these position/z-index tasks in a single issue, and moving your observations and required tasks to #3526123: <thead> element and CKEditor toolbar hang, when Toolbar is hidden, and closing this issue, since it only deals with a single scenario, and recent work is happening in the other issue?

justcaldwell’s picture

I think a couple things are getting conflated, neither of which is really an admin_toolbar issue.

1) z-index:
The overlap issue also appears if you're just using the core toolbar. See #3410871: The CKEditor 5 toolbar is overlapping with the Admin toolbar. Hopefully there will be a core fix at some point. In the meantime, we could mitigate the problem — @amit.mall was on the right track in #2. I would propose something like:

body:has(.ck.ck-editor) .toolbar-oriented .toolbar-bar {
  z-index: calc(var(--ck-z-panel) + 1);
}

2) Top offset:
When the editor area is focused, ckeditor is supposed to make the toolbar 'sticky' (with an appropriate offset), but #3487446: CKEditor5 sticky panel top offset does not get recalculated after calling Drupal.displace(). There might be something we could do to mitigate this, too, but I would argue any attempt should be handled in a separate issue.

justcaldwell’s picture

Status: Needs work » Needs review
StatusFileSize
new472 bytes

Opened an MR with the mitigation proposed in #16. Static patch attached.

ressa’s picture

Thanks for taking a closer look at this @justcaldwell, as well as refining @amit.mall's MR, your plan sounds great. Adding the related CKEditor index and position issues you shared issues as such, to connect them all.

neptune-dc’s picture

StatusFileSize
new4.65 MB

I can confirm that patch #18 works as intended.

neptune-dc’s picture

Status: Needs review » Reviewed & tested by the community
ressa’s picture

Issue summary: View changes

Adding @dydave's excellent image in the Issue Summary.

dydave’s picture

Status: Reviewed & tested by the community » Needs review

Back to Needs review after getting moved to RTBC at #21 to prevent MR from getting merged abruptly:

Please give us a little bit of time to review and test the changes. 🙂
But your feedback above and confirmation the changes worked as expected, were well received. 👌

Additionally, we'd like to squeeze in a few minor changes (wording, texts and such) in the next MR to get merged, so the opportunity to add another commit would be greatly appreciated by maintainers.

Any additional feedback, testing and comments would be welcome.
Thanks everyone for the great help on this issue! 🙏

dydave’s picture

Really sorry everyone for the delay on this. 😅

Just a quick update to let everyone know I have already reviewed and tested the MR and the changes suggested by Michael (@justcaldwell) above at #16 completely make sense.

I'll try to get this merged ASAP this week so this issue could be crossed off our list. 👌

Thanks again everyone for the great work, your patience and understanding. 🙏

dydave’s picture

We're very lucky we could have your help and attention Michael (@justcaldwell) on these kinds of tricky JS/CSS interactions and behaviors! 🙏

Thanks a lot for the very clear explanation above at #16 on the two issues with the z-index and the offset.

I've just updated the related Core issue, see:
#3410871-24: The CKEditor 5 toolbar is overlapping with the Admin toolbar
where I submitted the solution from the MR and asked where it could potentially be added in Core.

I suggest we wait a little bit longer and see if these changes could actually be moved to Core instead of Admin Toolbar, so it fixes the issue for everyone (Users and non users of this module 🙂), before moving forward with this merge request.

Once again, sorry for the delay on this, but I've been giving priority to other issues in the queue.
Let's try pushing together a bit more on the Core side, maybe?

Any feedback, questions or comments would be greatly appreciated.
Thanks again for all the great work! 👍