feat(comp): Add IntervalWrapper to interaction enhancers

[Kadee80]

overview

This PR adds IntervalWrapper to interaction enhancers. A follow up PR will be a refactor to replace all RefreshCards in the app with Cards wrapped in IntervalWrappers.

changelog

• feat(comp): Add IntervalWrapper to interaction enhancers

review requests

Standard review.

[mcous]

Great name 🐩

[codecov]

Codecov Report

Merging #1942 into edge will decrease coverage by 0.04%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##             edge    #1942      +/-   ##
==========================================
- Coverage   32.45%   32.41%   -0.05%     
==========================================
  Files         420      421       +1     
  Lines        6769     6778       +9     
==========================================
  Hits         2197     2197              
- Misses       4572     4581       +9

Impacted Files	Coverage Δ
...nents/src/interaction-enhancers/IntervalWrapper.js	0% <0%> (ø)

---

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 14a54a5...93b043e. Read the comment docs.

[IanLondon]

Cool! I'm not sure what it's for, but seems neat lol. I wonder if the child will want to have access to resetting or stopping the interval, but that's something you can add if it's needed.

[b-cooper]

👀 looks good, just one comment

[b-cooper]

This component looks good. I wonder though, if the intervalId would be more suited hung off of the class.

intervalId: ?IntervalId = null

cDM () {
  this.intervalId = setInterval(() => {this.props.refresh() }, this.props.interval)
}

cWU () {
  if (this.intervalId) {
    clearInterval(this.intervalId)
  }
}

Also, some circles would consider a direct call to this.setState in cDM an anti-pattern

[mcous]

Thanks for noting this! I just gave the react docs a quick review. Interval ID in state is nice for debugging, and setState in componentDidMount could be ok because the extra render triggered by the setState will not be seen by the user. Moving intervalID from state to the class itself would get rid of the double render but may be premature optimization? I guess the question is does the niceness of having it in state outweigh performance concerns?

[Kadee80]

I'm going to leave it in State for now. Once this wrapper is used in several places in the app and we can confirm is all good I'm happy to revisit this convo and optimize further.