Skip to content

Conversation

aaron-steinfeld
Copy link
Contributor

Description

Update table search debounce from 200ms to 400ms to reduce api calls.

Testing

Manual, updated UT

Checklist:

  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Any dependent changes have been merged and published in downstream modules

#1072

@aaron-steinfeld aaron-steinfeld requested a review from a team as a code owner August 18, 2021 19:33
@codecov
Copy link

codecov bot commented Aug 18, 2021

Codecov Report

Merging #1074 (3fb4632) into main (4ee6013) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1074      +/-   ##
==========================================
- Coverage   85.08%   85.08%   -0.01%     
==========================================
  Files         822      822              
  Lines       16931    16924       -7     
  Branches     2045     2045              
==========================================
- Hits        14406    14399       -7     
  Misses       2494     2494              
  Partials       31       31              
Impacted Files Coverage Δ
...nts/src/table/controls/table-controls.component.ts 77.14% <100.00%> (-2.08%) ⬇️

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 4ee6013...3fb4632. Read the comment docs.

@github-actions

This comment has been minimized.

jake-bassett
jake-bassett previously approved these changes Aug 18, 2021
this.checkboxDiffer = this.differFactory.find([]).create();

this.subscriptionLifecycle.add(
this.searchDebounceSubject.pipe(debounceTime(200)).subscribe(text => this.searchChange.emit(text))
Copy link
Contributor

@anandtiwary anandtiwary Aug 18, 2021

Choose a reason for hiding this comment

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

We can really use the ootb debounceTime property of search box component. Additional subscription here is not required in this table.

Please update the time in search box component as well either ways

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yeah I forgot we added that - switched over to using that one (searchbox doesn't have a built in time, it's not debouncing by default)

@github-actions

This comment has been minimized.

@aaron-steinfeld aaron-steinfeld merged commit 9b69822 into main Aug 18, 2021
@aaron-steinfeld aaron-steinfeld deleted the increase-default-debounce branch August 18, 2021 21:52
@github-actions
Copy link

Unit Test Results

    4 files  ±0  263 suites  ±0   16m 17s ⏱️ +42s
947 tests ±0  947 ✔️ ±0  0 💤 ±0  0 ❌ ±0 
954 runs  ±0  954 ✔️ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit 9b69822. ± Comparison against base commit 4ee6013.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants