Skip to content

Commit

Permalink
Tile: fix double insert animation
Browse files Browse the repository at this point in the history
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
  • Loading branch information
cguglielmo committed Aug 14, 2023
1 parent 21fcdf8 commit a6f6343
Show file tree
Hide file tree
Showing 8 changed files with 221 additions and 5 deletions.
5 changes: 5 additions & 0 deletions eclipse-scout-core/src/jquery/jquery-scout-types.ts
Expand Up @@ -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 {$}
Expand Down
4 changes: 4 additions & 0 deletions eclipse-scout-core/src/jquery/jquery-scout.js
Expand Up @@ -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);
};
Expand Down
8 changes: 8 additions & 0 deletions eclipse-scout-core/src/testing/jquery-testing.ts
Expand Up @@ -192,5 +192,13 @@ export const JQueryTesting = {
cancelable: true
});
$elem[0].dispatchEvent(event);
},

whenAnimationEnd($elem: JQuery): JQuery.Promise<Document> {
let def = $.Deferred();
$elem.oneAnimationEnd(() => {
def.resolve();
});
return def.promise();
}
};
6 changes: 4 additions & 2 deletions eclipse-scout-core/src/tile/Tile.less
Expand Up @@ -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();
}

Expand Down
4 changes: 2 additions & 2 deletions eclipse-scout-core/src/tile/TileGrid.ts
Expand Up @@ -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');
}
Expand Down
2 changes: 1 addition & 1 deletion eclipse-scout-core/src/tile/TileGridLayout.ts
Expand Up @@ -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

Expand Down
193 changes: 193 additions & 0 deletions eclipse-scout-core/test/tile/TileGridSpec.ts
Expand Up @@ -1214,4 +1214,197 @@ describe('TileGrid', () => {

});

describe('tile visibility', () => {
beforeEach(() => {
$(`<style>
@keyframes nop { 0% { opacity: 1; } 100% { opacity: 1; } }
.tile.animate-visible { animation: nop; animation-duration: 100ms;}
.tile.animate-invisible { animation: nop; animation-duration: 100ms;}
.tile.animate-insert { animation: nop; animation-duration: 100ms;}
.tile.animate-remove { animation: nop; animation-duration: 100ms;}
.tile.newly-rendered { visibility: hidden !important;}
.tile.before-animate-insert { visibility: hidden !important;}
</style>`).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);
});
});
});
4 changes: 4 additions & 0 deletions karma-jasmine-scout/src/JasmineScout.css
Expand Up @@ -10,3 +10,7 @@
.hidden {
display: none !important; /* NOSONAR (!important is okay here) */
}

.invisible {
visibility: hidden !important; /* NOSONAR (!important is okay here) */
}

0 comments on commit a6f6343

Please sign in to comment.