fix viewCache for gridList (resolves issue #32) #33
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
ghenry22
changed the title
lVlyke
reviewed
Jul 15, 2024
| let itemsPerRow = gridList ? Math.floor(scrollContainer.clientWidth / itemWidth!) : 1; | ||
|
|
||
| // Don't reset items per row to 0 on view change as it invalidates the viewCache | ||
| if (itemsPerRow === 0 && this._itemsPerRow > 0) { |
There was a problem hiding this comment.
We should be able to safely skip running all of the logic in this subscription when the list element has a width or height of 0. Instead of checking for itemsPerRow being 0 here, we could update the filter condition on line 304 to additionally check that the clientWidth and clientHeight of scrollContainer are both greater than 0.
This should hopefully result in the same fixed behavior and also avoid updating other state variables unnecessarily.
There was a problem hiding this comment.
I'll test it out and see if the behaviour is the same, will update the PR accordingly, thanks
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.
Resolves #32
If you look at the itemsPerRow calculation you can see that for listview it is always 1. For grid view it is a calculation that relies upon clientWidth.
When you navigate from a list to a detail page for example, the virtualscroll element is hidden so clientWidth changes to 0. When you navigate back to your list you get a scroll to a random location and don't benefit from the view cache as the change in itemsPerRow from x to 0 and back to x as the virtualscroll element goes visible -> invisible -> visible again causes a change and invalidates the cache.
Only applies when the private _itemsPerRow is already a value greater than 0.
Have tested in a couple of my own apps and it's working great now when I navigate back from a detail view with gridlist enabled I get the cached view exactly as it was which is perfect! Have not seen any issues or side effects and wouldn't expect to as this only catches the case where clientWidth goes to 0, in which case the old logic would show nothing anyway.