Merge pull request #17123 from ckeditor/ck/17067

Fix (ui): Fix scrolling in dropdowns when block toolbar button is active. Closes #17067

packages/ckeditor5-ui/package.json

@@ -48,6 +48,7 @@
     "@ckeditor/ckeditor5-table": "43.1.0",
     "@ckeditor/ckeditor5-typing": "43.1.0",
     "@ckeditor/ckeditor5-undo": "43.1.0",
+    "@ckeditor/ckeditor5-code-block": "43.1.0",
     "@types/color-convert": "2.0.0",
     "typescript": "5.0.4",
     "webpack": "^5.94.0",

packages/ckeditor5-ui/src/toolbar/block/blocktoolbar.ts

@@ -15,6 +15,8 @@ import {
 } from '@ckeditor/ckeditor5-core';
 
 import {
+	type EventInfo,
+	getAncestors,
 	global,
 	Rect,
 	ResizeObserver,
@@ -446,11 +448,25 @@ export default class BlockToolbar extends Plugin {
 		let pendingAnimationFrame = false;
 
 		// Reposition the button on scroll, but do it only once per animation frame to avoid performance issues.
-		const repositionOnScroll = () => {
+		const repositionOnScroll = ( evt: EventInfo, domEvt: Event ) => {
 			if ( pendingAnimationFrame ) {
 				return;
 			}
 
+			// It makes no sense to reposition the button when the user scrolls the dropdown or any other
+			// nested scrollable element. The button should be repositioned only when the user scrolls the
+			// editable or any other scrollable parent of the editable. Leaving it as it is buggy on Chrome
+			// where scrolling nested scrollables is not properly handled.
+			// See more: https://github.com/ckeditor/ckeditor5/issues/17067
+			const editableElement = this._getSelectedEditableElement();
+
+			if (
+				domEvt.target !== global.document &&
+				!getAncestors( editableElement ).includes( domEvt.target as HTMLElement )
+			) {
+				return;
+			}
+
 			pendingAnimationFrame = true;
 			global.window.requestAnimationFrame( () => {
 				this._updateButton();

packages/ckeditor5-ui/tests/manual/blocktoolbar/blocktoolbar.js

@@ -8,6 +8,7 @@
 import BalloonEditor from '@ckeditor/ckeditor5-editor-balloon/src/ballooneditor.js';
 import Essentials from '@ckeditor/ckeditor5-essentials/src/essentials.js';
 import List from '@ckeditor/ckeditor5-list/src/list.js';
+import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock.js';
 import Image from '@ckeditor/ckeditor5-image/src/image.js';
 import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption.js';
 import { Paragraph, ParagraphButtonUI } from '@ckeditor/ckeditor5-paragraph';
@@ -17,9 +18,12 @@ import BlockToolbar from '../../../src/toolbar/block/blocktoolbar.js';
 
 BalloonEditor
 	.create( document.querySelector( '#editor' ), {
-		plugins: [ Essentials, List, Paragraph, Heading, Image, ImageCaption, HeadingButtonsUI, ParagraphButtonUI, BlockToolbar ],
+		plugins: [
+			Essentials, List, Paragraph, Heading, Image, ImageCaption,
+			HeadingButtonsUI, ParagraphButtonUI, BlockToolbar, CodeBlock
+		],
 		blockToolbar: [
-			'paragraph', 'heading1', 'heading2', 'heading3', 'bulletedList', 'numberedList', 'paragraph',
+			'paragraph', 'heading1', 'heading2', 'heading3', 'bulletedList', 'numberedList', 'paragraph', 'codeBlock',
 			'heading1', 'heading2', 'heading3', 'bulletedList', 'numberedList', 'paragraph', 'heading1', 'heading2', 'heading3',
 			'bulletedList', 'numberedList'
 		]

packages/ckeditor5-ui/tests/manual/blocktoolbar/scrollable-blocktoolbar.js

@@ -10,6 +10,7 @@ import Essentials from '@ckeditor/ckeditor5-essentials/src/essentials.js';
 import List from '@ckeditor/ckeditor5-list/src/list.js';
 import Image from '@ckeditor/ckeditor5-image/src/image.js';
 import ImageCaption from '@ckeditor/ckeditor5-image/src/imagecaption.js';
+import CodeBlock from '@ckeditor/ckeditor5-code-block/src/codeblock.js';
 import { Paragraph, ParagraphButtonUI } from '@ckeditor/ckeditor5-paragraph';
 import Heading from '@ckeditor/ckeditor5-heading/src/heading.js';
 import HeadingButtonsUI from '@ckeditor/ckeditor5-heading/src/headingbuttonsui.js';
@@ -26,9 +27,12 @@ createBlockButtonEditor( '#editor-scrollable' ).then( editor => {
 function createBlockButtonEditor( element ) {
 	return BalloonEditor
 		.create( document.querySelector( element ), {
-			plugins: [ Essentials, List, Paragraph, Heading, Image, ImageCaption, HeadingButtonsUI, ParagraphButtonUI, BlockToolbar ],
+			plugins: [
+				Essentials, List, Paragraph, Heading, Image, ImageCaption,
+				HeadingButtonsUI, ParagraphButtonUI, BlockToolbar, CodeBlock
+			],
 			blockToolbar: [
-				'paragraph', 'heading1', 'heading2', 'heading3', 'bulletedList', 'numberedList', 'paragraph',
+				'paragraph', 'heading1', 'heading2', 'heading3', 'bulletedList', 'numberedList', 'paragraph', 'codeBlock',
 				'heading1', 'heading2', 'heading3', 'bulletedList', 'numberedList', 'paragraph', 'heading1', 'heading2', 'heading3',
 				'bulletedList', 'numberedList'
 			]

packages/ckeditor5-ui/tests/toolbar/block/blocktoolbar.js

@@ -914,6 +914,29 @@ describe( 'BlockToolbar', () => {
 
 			expect( spy ).to.be.calledOnce;
 		} );
+
+		it( 'should not _call _clipButtonToViewport when event target is not editable ancestor', () => {
+			const spy = sinon.spy( blockToolbar, '_clipButtonToViewport' );
+
+			buttonView.isVisible = true;
+
+			document.body.dispatchEvent( new Event( 'scroll' ) );
+			clock.tick( 100 );
+			expect( spy ).to.be.called;
+
+			spy.resetHistory();
+
+			// Create a fake parent element and dispatch scroll event on it.
+			// It's not a button ancestor so _clipButtonToViewport should not be called.
+			const evt = new Event( 'scroll' );
+			const fakeParent = document.createElement( 'div' );
+
+			sinon.stub( evt, 'target' ).value( fakeParent );
+			document.body.dispatchEvent( evt );
+			clock.tick( 100 );
+			fakeParent.remove();
+			expect( spy ).not.to.be.called;
+		} );
 	} );
 
 	describe( '_clipButtonToViewport()', () => {