From a6f634362068fc4a7766cbc5dbd5949005457810 Mon Sep 17 00:00:00 2001 From: Claudio Guglielmo Date: Wed, 9 Aug 2023 21:10:33 +0200 Subject: [PATCH] Tile: fix double insert animation Both, the visible and the insert animation, set the tile to invisible before the animation starts. In addition, the insert animation waits for the tile grid layout to finish before it starts. The tile grid layout may position the tiles with an animation, e.g. move the tiles away to make space for the tile to be inserted. If a tile is inserted and a tile made visible while the tile grid layout is still positioning the tiles, the new tile flickers because tile._renderVisible() makes it visible even though the insert animation is still pending and the tile should stay invisible. Can be reproduced with the tile grid in the classic widgets app: 1. Add a filter in the source code with addFilter(() -> true); 2. Open Tile Field 3. Sort descending 4. Insert a new tile The filter is necessary because the server will send a tiles property change event first and then a filteredTiles property change event. When the new tile is inserted, the existing filter does not know it yet and makes it invisible. When the filteredTiles event is processed, the tile is made visible again which leads to the described behavior. 350043 --- .../src/jquery/jquery-scout-types.ts | 5 + eclipse-scout-core/src/jquery/jquery-scout.js | 4 + .../src/testing/jquery-testing.ts | 8 + eclipse-scout-core/src/tile/Tile.less | 6 +- eclipse-scout-core/src/tile/TileGrid.ts | 4 +- eclipse-scout-core/src/tile/TileGridLayout.ts | 2 +- eclipse-scout-core/test/tile/TileGridSpec.ts | 193 ++++++++++++++++++ karma-jasmine-scout/src/JasmineScout.css | 4 + 8 files changed, 221 insertions(+), 5 deletions(-) diff --git a/eclipse-scout-core/src/jquery/jquery-scout-types.ts b/eclipse-scout-core/src/jquery/jquery-scout-types.ts index f93a3a23db9..468462f2c58 100644 --- a/eclipse-scout-core/src/jquery/jquery-scout-types.ts +++ b/eclipse-scout-core/src/jquery/jquery-scout-types.ts @@ -580,6 +580,11 @@ declare global { */ isDisplayNone(): boolean; + /** + * returns true of the current element has the 'visibility' property set to 'hidden'. + */ + isVisibilityHidden(): boolean; + /** * @param tabbable true, to make the component tabbable. False, to make it neither tabbable nor focusable. * @returns {$} diff --git a/eclipse-scout-core/src/jquery/jquery-scout.js b/eclipse-scout-core/src/jquery/jquery-scout.js index 2c79fea6949..1b95f097e4b 100644 --- a/eclipse-scout-core/src/jquery/jquery-scout.js +++ b/eclipse-scout-core/src/jquery/jquery-scout.js @@ -731,6 +731,10 @@ $.fn.isDisplayNone = function() { return this.css('display') === 'none'; }; +$.fn.isVisibilityHidden = function() { + return this.css('visibility') === 'hidden'; +}; + $.fn.setTabbable = function(tabbable) { return this.attr('tabIndex', tabbable ? 0 : null); }; diff --git a/eclipse-scout-core/src/testing/jquery-testing.ts b/eclipse-scout-core/src/testing/jquery-testing.ts index 7522609cbb0..f0bf00fbb54 100644 --- a/eclipse-scout-core/src/testing/jquery-testing.ts +++ b/eclipse-scout-core/src/testing/jquery-testing.ts @@ -192,5 +192,13 @@ export const JQueryTesting = { cancelable: true }); $elem[0].dispatchEvent(event); + }, + + whenAnimationEnd($elem: JQuery): JQuery.Promise { + let def = $.Deferred(); + $elem.oneAnimationEnd(() => { + def.resolve(); + }); + return def.promise(); } }; diff --git a/eclipse-scout-core/src/tile/Tile.less b/eclipse-scout-core/src/tile/Tile.less index a9d269a7352..f91b7916f80 100644 --- a/eclipse-scout-core/src/tile/Tile.less +++ b/eclipse-scout-core/src/tile/Tile.less @@ -8,8 +8,10 @@ * SPDX-License-Identifier: EPL-2.0 */ .tile { - &.newly-rendered { - /* Tile should be invisible until it has the correct position, otherwise it might be visible in the top left corner in the time between rendering and layouting */ + // Tile should be invisible until it has the correct position, otherwise it might be visible in the top left corner in the time between rendering and layouting + &.newly-rendered, + // Use a dedicated invisible class for the insert operation to not get in a mess with the invisible class from the tile.visible state + &.before-animate-insert { .invisible(); } diff --git a/eclipse-scout-core/src/tile/TileGrid.ts b/eclipse-scout-core/src/tile/TileGrid.ts index fa17182d081..709dc312194 100644 --- a/eclipse-scout-core/src/tile/TileGrid.ts +++ b/eclipse-scout-core/src/tile/TileGrid.ts @@ -373,13 +373,13 @@ export class TileGrid extends Widget implements TileGridModel { if (!tile.rendered) { return; } - tile.$container.addClass('invisible'); + tile.$container.addClass('before-animate-insert'); // Wait until the layout animation is done before animating the insert operation. // Also make them invisible to not cover existing tiles while they are moving or changing size. // Also do it for tiles which don't have an insert animation (e.g. placeholders), due to the same reason. this.one('layoutAnimationDone', () => { if (tile.rendered) { - tile.$container.removeClass('invisible'); + tile.$container.removeClass('before-animate-insert'); if (this._animateTileInsertion(tile)) { tile.$container.addClassForAnimation('animate-insert'); } diff --git a/eclipse-scout-core/src/tile/TileGridLayout.ts b/eclipse-scout-core/src/tile/TileGridLayout.ts index 61eeafe6a0f..da26e47fbdf 100644 --- a/eclipse-scout-core/src/tile/TileGridLayout.ts +++ b/eclipse-scout-core/src/tile/TileGridLayout.ts @@ -205,7 +205,7 @@ export class TileGridLayout extends LogicalGridLayout { // (e.g. if it is not in the viewport anymore). In that case the animation must be stopped otherwise it may be placed at a wrong position tile.$container.stop(); - if (tile.$container.hasClass('invisible') || tile.$container.hasClass('animate-visible')) { + if (tile.$container.isVisibilityHidden() || tile.$container.hasClass('animate-visible')) { // When tiles are inserted they are invisible because a dedicated insert animation will be started after the layouting, // the animation here is to animate the position change -> don't animate inserted tiles here diff --git a/eclipse-scout-core/test/tile/TileGridSpec.ts b/eclipse-scout-core/test/tile/TileGridSpec.ts index 586dc6784ae..402a70f9362 100644 --- a/eclipse-scout-core/test/tile/TileGridSpec.ts +++ b/eclipse-scout-core/test/tile/TileGridSpec.ts @@ -1214,4 +1214,197 @@ describe('TileGrid', () => { }); + describe('tile visibility', () => { + beforeEach(() => { + $(``).appendTo($('#sandbox')); + }); + + it('is correct after insert animation', async () => { + let tileGrid = createTileGrid(0); + tileGrid.render(); + tileGrid.insertTile({objectType: Tile}); + let tile = tileGrid.tiles[0]; + expect(tile.$container).not.toHaveClass('animate-insert'); + expect(tile.$container.isVisibilityHidden()).toBe(true); // not visible until layout is done + expect(tile.$container.isVisible()).toBe(true); + + tileGrid.validateLayout(); + expect(tile.$container).toHaveClass('animate-insert'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + await JQueryTesting.whenAnimationEnd(tile.$container); + expect(tile.$container).not.toHaveClass('animate-insert'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + }); + + it('is correct after hide animation', async () => { + let tileGrid = createTileGrid(1); + let tile = tileGrid.tiles[0]; + tileGrid.render(); + tileGrid.validateLayout(); + expect(tile.$container).not.toHaveClass('animate-invisible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + tile.setVisible(false); + expect(tile.$container).toHaveClass('animate-invisible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + await JQueryTesting.whenAnimationEnd(tile.$container); + expect(tile.$container).not.toHaveClass('animate-invisible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(false); + }); + + it('is correct after show animation', async () => { + let tileGrid = createTileGrid(1); + let tile = tileGrid.tiles[0]; + tile.setVisible(false); + tileGrid.render(); + tileGrid.validateLayout(); + expect(tile.$container).not.toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(false); + + tile.setVisible(true); + expect(tile.$container).not.toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(true); // not visible until layout is done + expect(tile.$container.isVisible()).toBe(true); + + tileGrid.validateLayoutTree(); + expect(tile.$container).toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + await JQueryTesting.whenAnimationEnd(tile.$container); + expect(tile.$container).not.toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + }); + + it('is correct after hide > show animation', async () => { + let tileGrid = createTileGrid(1); + let tile = tileGrid.tiles[0]; + tileGrid.render(); + tileGrid.validateLayout(); + + tile.setVisible(false); + expect(tile.$container).toHaveClass('animate-invisible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + // Animation is not complete yet + await sleep(10); + expect(tile.$container).toHaveClass('animate-invisible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + // Make it visible again while hide animation still runs + tile.setVisible(true); + expect(tile.$container).not.toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(true); // not visible until layout is done + expect(tile.$container.isVisible()).toBe(true); + + tileGrid.validateLayoutTree(); + expect(tile.$container).toHaveClass('animate-visible'); + expect(tile.$container).not.toHaveClass('animate-invisible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + await JQueryTesting.whenAnimationEnd(tile.$container); + expect(tile.$container).not.toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + }); + + it('is correct after show > hide animation', async () => { + let tileGrid = createTileGrid(1); + let tile = tileGrid.tiles[0]; + tile.setVisible(false); + tileGrid.render(); + tileGrid.validateLayout(); + tile.setVisible(true); + + expect(tile.$container).not.toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(true); // not visible until layout is done + expect(tile.$container.isVisible()).toBe(true); + + tileGrid.validateLayoutTree(); + expect(tile.$container).toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + // Animation is not complete yet + await sleep(10); + expect(tile.$container).toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + tile.setVisible(false); + expect(tile.$container).toHaveClass('animate-invisible'); + expect(tile.$container).not.toHaveClass('animate-visible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + + await JQueryTesting.whenAnimationEnd(tile.$container); + expect(tile.$container).not.toHaveClass('animate-invisible'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(false); + }); + + it('is correct after insert > hide > show animation', async () => { + let tileGrid = createTileGrid(1); + tileGrid.render(); + tileGrid.validateLayout(); + + tileGrid.setTiles([{objectType: Tile}, tileGrid.tiles[0]]); + let tile = tileGrid.tiles[0]; + expect(tile.$container).not.toHaveClass('animate-insert'); + expect(tile.$container.isVisibilityHidden()).toBe(true); + + tile.setVisible(false); + tile.setVisible(true); + expect(tile.$container.isVisibilityHidden()).toBe(true); + + // During the tile grid layout, the inserted tile must not be visible because the insert animation has not been started yet, even if tile.setVisible(true) was called + // The layout animation needs a real viewport and sizes -> To make it easier in the test setup we suppress the layoutAnimationDone event to delay the start of the insert animation + let triggerSpy = spyOn(tileGrid, 'trigger'); + let suppressedEvent; + triggerSpy.and.callFake((type, event): any => { + if (type === 'layoutAnimationDone') { + suppressedEvent = event; + } else { + triggerSpy.and.callThrough(); + } + }); + tileGrid.validateLayout(); + tileGrid.validateLayoutTree(); // Triggers the scheduled post validate task in Tile._renderVisible + expect(tile.$container.isVisibilityHidden()).toBe(true); + expect(tile.$container.isVisible()).toBe(true); + + // Finish TileGridLayout -> Insert animation will start + triggerSpy.and.callThrough(); + tileGrid.trigger('layoutAnimationDone', suppressedEvent); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container).toHaveClass('animate-insert'); + expect(tile.$container.isVisible()).toBe(true); + + await JQueryTesting.whenAnimationEnd(tile.$container); + expect(tile.$container).not.toHaveClass('animate-insert'); + expect(tile.$container.isVisibilityHidden()).toBe(false); + expect(tile.$container.isVisible()).toBe(true); + }); + }); }); diff --git a/karma-jasmine-scout/src/JasmineScout.css b/karma-jasmine-scout/src/JasmineScout.css index 7141e972c8e..d2949aef993 100644 --- a/karma-jasmine-scout/src/JasmineScout.css +++ b/karma-jasmine-scout/src/JasmineScout.css @@ -10,3 +10,7 @@ .hidden { display: none !important; /* NOSONAR (!important is okay here) */ } + +.invisible { + visibility: hidden !important; /* NOSONAR (!important is okay here) */ +}