ui: Alter position of logic for showing the Round Trip Time tab to prevent DOM refresh #7377
Conversation
This file contains hidden or 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
Previously we checked the length of tomography.distances to decide whether to show the RTT tab or not. Previous to our ember upgrade this would not cause a DOM reload of so many elements (i.e. all of the tab content). Since our ember upgrade, any change to tomography (so not necessarily the length of distances) seems to fire a change to the length (even if the length remains the same). The knock on effect of this is that the array of tab panels seems to be recalculated (but remain the same) and all of the tab panels are completely re-rendered, causing the scroll of the page to be reset. This commit moves the check for tomography.distance.length to the lower down to the partial, which means the array of tab panels always remains the same, which consequently means that the entire array of tab panels is never re-rendered entirely, and therefore fixes the issue.
|
Sorry @kaxcode , I had a re-think about this and decided to put the logic in the same template it originally was (which is also the same place we have the logic for showing the tab or not). The other benefit is, my previous attempt would still show the |
johncowen
changed the title
freddygv
pushed a commit
that referenced
this pull request
Mar 12, 2020
…event DOM refresh (#7377) * ui: Move tomography length check inside of the partial Previously we checked the length of tomography.distances to decide whether to show the RTT tab or not. Previous to our ember upgrade this would not cause a DOM reload of so many elements (i.e. all of the tab content). Since our ember upgrade, any change to tomography (so not necessarily the length of distances) seems to fire a change to the length (even if the length remains the same). The knock on effect of this is that the array of tab panels seems to be recalculated (but remain the same) and all of the tab panels are completely re-rendered, causing the scroll of the page to be reset. This commit moves the check for tomography.distance.length to the lower down with the loop, which means the array of tab panels always remains the same, which consequently means that the entire array of tab panels is never re-rendered entirely, and therefore fixes the issue.
alvin-huang
pushed a commit
to alvin-huang/consul
that referenced
this pull request
May 6, 2020
…event DOM refresh (hashicorp#7377) * ui: Move tomography length check inside of the partial Previously we checked the length of tomography.distances to decide whether to show the RTT tab or not. Previous to our ember upgrade this would not cause a DOM reload of so many elements (i.e. all of the tab content). Since our ember upgrade, any change to tomography (so not necessarily the length of distances) seems to fire a change to the length (even if the length remains the same). The knock on effect of this is that the array of tab panels seems to be recalculated (but remain the same) and all of the tab panels are completely re-rendered, causing the scroll of the page to be reset. This commit moves the check for tomography.distance.length to the lower down with the loop, which means the array of tab panels always remains the same, which consequently means that the entire array of tab panels is never re-rendered entirely, and therefore fixes the issue.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Previously we checked the length of tomography.distances to decide
whether to show the RTT tab or not. Previous to our ember upgrade this
would not cause a DOM reload of so many elements (i.e. all of the tab
content). Since our ember upgrade, any change to tomography (so not
necessarily the length of distances) seems to fire a change to the length (even if
the length remains the same). The knock on effect of this is that the
array of tab panels seems to be recalculated (but remain the same) and
all of the tab panels are completely re-rendered, causing the scroll of
the page to be reset.
This commit moves the check for tomography.distance.length to the lower
down to the partial, which means the array of tab panels always remains
the same, which consequently means that the entire array of tab panels
is never re-rendered entirely, and therefore fixes the issue.
Fixes #7365
There is also another bug we found on our travels here, I decided to separate both PRs out, so theres another related one to follow.