blackbaud / skyux1

DEPRECATED This site contains the codebase for the AngularJS (1.x) implementation of the SKY UX framework. We no longer support this version of SKY UX, and we recommend that you use the latest version instead. https://developer.blackbaud.com/skyux/
MIT License
51 stars 68 forks source link

Do not layout tiles when a new tile is added to an existing view - Local #1086

Open Blackbaud-MatthewBell opened 6 years ago

Blackbaud-MatthewBell commented 6 years ago

The objective of this change is to reduce the occassions when a tile is moved in the DOM, as the act of moving a DOM element has negative consequences when it contains an iFrame

(https://stackoverflow.com/questions/8318264/how-to-move-an-iframe-in-the-dom-without-losing-its-state).

Currently when a Tile directive is added into a tile view, the initialization of that tile directive forces the dashboard to layout the tiles again. A common scenario for this is a tile-view that performs logic/security checks before adding the tile directive. Layout of the tiles in this case is unnecessary. Even though the tile itself is new, the tile-view element that contains the tile already exists and was initially layed out properly.

The reason the tile initialization forces the dashboard to layout the tiles is due to the issue #182. In this case the page $state is changing, impacting the actual tile-view elements. When the $state changes and a view now exists that previously did not, the tile-view element is recreated and it is placed back at its original location. This behavior occurs from the ui-view directive. In this case, the tile does need to be layed out again.

This change differentiates these scenarios by adding an attribute to the view element to determine if it has been layed out or not.

codecov-io commented 6 years ago

Codecov Report

Merging #1086 into master will not change coverage. The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1086   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         174     174           
  Lines        6528    6506   -22     
  Branches     1153    1148    -5     
======================================
- Hits         6528    6506   -22
Impacted Files Coverage Δ
js/sky/src/tiles/tiles.js 100% <100%> (ø) :arrow_up:
js/sky/src/modal/modal.factory.js 100% <0%> (ø) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bd3f5ea...ddd3c31. Read the comment docs.

Blackbaud-BobbyEarl commented 6 years ago

Hey @Blackbaud-MatthewBell thanks for the PR! Any suggestions on skyux1 consumers that might be able to review this for you?

Blackbaud-MatthewBell commented 6 years ago

Maybe someone from FE? I don't know that any consumers have dug much into the depths of Tile Dashboard. I think only me, Paul, and Patrick have been in that code.