This repository has been archived by the owner on Dec 8, 2022. It is now read-only.
Do not layout tiles when a new tile is added to an existing view - Local #1086
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 Report
@@ Coverage Diff @@
## master #1086 +/- ##
======================================
Coverage 100% 100%
======================================
Files 174 174
Lines 6528 6506 -22
Branches 1153 1148 -5
======================================
- Hits 6528 6506 -22
Continue to review full report at Codecov.
|
|
Hey @Blackbaud-MatthewBell thanks for the PR! Any suggestions on skyux1 consumers that might be able to review this for you? |
|
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. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.