Replace jQuery show animation with CSS animation in sidebar

The sidebar-showing animation was relying on jQuery, which just animates the width/height properties (absolutely killer for performance). Replace this with a CSS animation. Note, the animation isn't identical; previously the document size would animate, but this now just snaps to the destination size. Animating document size changes is unlikely to be viable any time soon, but we may want to consider some kind of 2-part move + resize animation. Signed-off-by: Chris Lord <[email protected]> Change-Id: I68eb16e9eff9a3a9601f3f903f9edb25b97a56cd
CollaboraOnline · Jan 31, 2025 · 2a318fc · 2a318fc
1 parent b444274
commit 2a318fc
Show file tree

Hide file tree

Showing 7 changed files with 29 additions and 13 deletions.
diff --git a/browser/css/cool.css b/browser/css/cool.css
@@ -329,6 +329,21 @@ body {
 	overflow: hidden;
 	z-index: 990;
 }
+
+#sidebar-dock-wrapper.visible {
+	display: block;
+	animation: 200ms ease-out appear-from-right;
+}
+
+@keyframes appear-from-right {
+	from {
+		transform: translateX(100%);
+	}
+	to {
+		transform: translateX(0);
+	}
+}
+
 #sidebar-panel {
 	padding: 0px;
 	margin: 0px;

diff --git a/browser/src/control/Control.Sidebar.ts b/browser/src/control/Control.Sidebar.ts
@@ -63,11 +63,11 @@ class Sidebar {
 	}
 
 	isVisible(): boolean {
-		return $('#sidebar-dock-wrapper').is(':visible');
+		return $('#sidebar-dock-wrapper').hasClass('visible');
 	}
 
 	closeSidebar() {
-		$('#sidebar-dock-wrapper').hide();
+		$('#sidebar-dock-wrapper').removeClass('visible');
 		this.map._onResize();
 
 		if (!this.map.editorHasFocus()) {
@@ -236,8 +236,7 @@ class Sidebar {
 				}
 
 				this.builder.build(this.container, [sidebarData]);
-				if (!this.isVisible())
-					$('#sidebar-dock-wrapper').show(this.options.animSpeed);
+				if (!this.isVisible()) $('#sidebar-dock-wrapper').addClass('visible');
 
 				this.map.uiManager.setDocTypePref('ShowSidebar', true);
 			} else {

diff --git a/cypress_test/integration_tests/common/helper.js b/cypress_test/integration_tests/common/helper.js
@@ -1106,7 +1106,7 @@ function getBlinkingCursorPosition(aliasName) {
 	var cursorSelector = '.cursor-overlay .blinking-cursor';
 	cy.cGet(cursorSelector).then(function(cursor) {
 		var boundRect = cursor[0].getBoundingClientRect();
-		var xPos = boundRect.right;
+		var xPos = (boundRect.left + boundRect.right) / 2;
 		var yPos = (boundRect.top + boundRect.bottom) / 2;
 		cy.wrap({x: xPos, y: yPos}).as(aliasName);
 	});

diff --git a/cypress_test/integration_tests/desktop/calc/scrolling_spec.js b/cypress_test/integration_tests/desktop/calc/scrolling_spec.js
@@ -73,7 +73,7 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Scroll through document',
 		}
 
 		// Document should scroll
-		desktopHelper.assertScrollbarPosition('horizontal', 80, 110);
+		desktopHelper.assertScrollbarPosition('horizontal', 115, 145);
 	});
 
 	it('Scroll while selecting with mouse', function () {

diff --git a/cypress_test/integration_tests/desktop/calc/sheet_switch_spec.js b/cypress_test/integration_tests/desktop/calc/sheet_switch_spec.js
@@ -30,12 +30,12 @@ describe(['tagdesktop', 'tagnextcloud', 'tagproxy'], 'Sheet switching tests', fu
 	it('Check view position on repeated selection of currently selected sheet', function() {
 		// initially we are on sheet 2 tab
 		cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'F720');
-		desktopHelper.assertScrollbarPosition('vertical', 260, 300);
+		desktopHelper.assertScrollbarPosition('vertical', 300, 340);
 
 		// click on sheet 2 tab (yes, current one)
 		cy.cGet('#spreadsheet-tab1').click();
 		cy.cGet(helper.addressInputSelector).should('have.prop', 'value', 'F720');
-		desktopHelper.assertScrollbarPosition('vertical', 220, 285);
+		desktopHelper.assertScrollbarPosition('vertical', 300, 340);
 
 		// go to different place in the spreadsheet
 		cy.cGet(helper.addressInputSelector).type('{selectAll}A2{enter}');

diff --git a/cypress_test/integration_tests/desktop/writer/editable_area_spec.js b/cypress_test/integration_tests/desktop/writer/editable_area_spec.js
@@ -211,12 +211,14 @@ describe(['taga11yenabled'], 'Editable area - Basic typing and caret moving', fu
 		ceHelper.type('{enter}');
 		ceHelper.type('green red');
 		// Need to wait here after typing paragraph in
-		// order for caret position to stick later
-		cy.wait(200);
+		// order for caret position to stick later.
+		// FIXME: We shouldn't need to wait.
+		cy.wait(1000);
 		ceHelper.checkPlainContent('green red');
 		ceHelper.moveCaret('left', '', 4);
 		ceHelper.checkCaretPosition(5);
 		helper.getBlinkingCursorPosition('P2');
+		cy.wait(800);
 		// move up to paragraph 3
 		ceHelper.moveCaret('up');
 		ceHelper.checkPlainContent('');

diff --git a/cypress_test/integration_tests/multiuser/writer/cursor_spec.js b/cypress_test/integration_tests/multiuser/writer/cursor_spec.js
@@ -23,18 +23,18 @@ describe(['tagmultiuser'], 'Check cursor and view behavior', function() {
 		cy.cSetActiveFrame('#iframe1');
 		searchHelper.typeIntoSearchField('P'); // avoid focus loss
 		searchHelper.typeIntoSearchField('Pellentesque porttitor');
-		desktopHelper.assertScrollbarPosition('vertical', 375, 385);
+		desktopHelper.assertScrollbarPosition('vertical', 380, 390);
 
 		// verify that second view is scrolled to the editor
 		cy.cSetActiveFrame('#iframe2');
-		desktopHelper.assertScrollbarPosition('vertical', 375, 385);
+		desktopHelper.assertScrollbarPosition('vertical', 380, 390);
 
 		// now move cursor a bit in the first view
 		cy.cSetActiveFrame('#iframe1');
 		cy.cGet('#map').type('{downArrow}{downArrow}{downArrow}{downArrow}{downArrow}{downArrow}');
 
 		// verify that second view is still at the same position (no jump)
 		cy.cSetActiveFrame('#iframe2');
-		desktopHelper.assertScrollbarPosition('vertical', 375, 385);
+		desktopHelper.assertScrollbarPosition('vertical', 380, 390);
 	});
 });