Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Fixed issues around the data providers being called too many times. #20

Merged
merged 9 commits into from
Mar 7, 2019

Conversation

Blackbaud-TrevorBurch
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Jan 25, 2019

Codecov Report

Merging #20 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master      #20      +/-   ##
==========================================
+ Coverage   99.68%   99.69%   +0.01%     
==========================================
  Files          18       18              
  Lines         314      330      +16     
  Branches       38       42       +4     
==========================================
+ Hits          313      329      +16     
  Misses          1        1
Impacted Files Coverage Δ
...modules/list-view-grid/list-view-grid.component.ts 99.33% <100%> (+0.07%) ⬆️

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 7fba23a...7a42cdf. Read the comment docs.

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch changed the title Fixed issues around the date providers being called too many times. Fixed issues around the data providers being called too many times. Feb 1, 2019
@Blackbaud-SteveBrush
Copy link
Member

@Blackbaud-TrevorBurch Is there an issue or detailed description we can add to the pull request to better explain the need for these changes?

@Blackbaud-TrevorBurch
Copy link
Member Author

@Blackbaud-SteveBrush https://github.com/blackbaud/skyux-list-builder/issues/12 I had linked it through Zenhub but didn't put it in the description.

There are two issues being fixed here in reality.

  1. The grid state observable was firing out of order causing extra actions to be sent. I added a watch for the lastUpdate time to ensure this doesn't happen.
  2. The grid was firing the column change event too many times and thus we were firing actions to often. I've fixed this in grid but also added a check here to ensure that we only fire an action to change the columns if it is actual necessary (i.e. I check to make sure the new id array is different from the previous one)

Copy link
Contributor

@Blackbaud-AlexKingman Blackbaud-AlexKingman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for tracking this down.

@Blackbaud-TrevorBurch
Copy link
Member Author

@Blackbaud-SteveBrush I added some unit tests for these changes

@Blackbaud-TrevorBurch Blackbaud-TrevorBurch merged commit 6d7fdf1 into master Mar 7, 2019
@Blackbaud-TrevorBurch Blackbaud-TrevorBurch deleted the multiple-get-data-calls branch March 7, 2019 17:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants