Add config options for form control #24
Conversation
evansiroky
commented
Apr 4, 2017
evansiroky
commented
- allow debouncing of trip plan requests
- add option to not auto-plan trips
* allow debouncing of trip plan requests * add option to not auto-plan trips
lib/actions/form.js
Outdated
| } | ||
| } | ||
|
|
||
| let debouncedPlanTrip // store as global, so it can be reused. Maybe there's a better way? |
There was a problem hiding this comment.
@trevorgerhardt let me now if there is a better way to do this. Since a new function is returned on each formChanged event I put the debouncedPlanTrip variable in this scope so it would be available in subsequent calls.
There was a problem hiding this comment.
If debouncePlanTimeMs can change then you would need to recreate the debounced function when it does. If it can't change then it's application configuration and shouldn't be retrieved within the function and can be created once with dispatch passed in as a parameter:
const debouncedPlanTrip = debounce(({dispatch}) => dispatch(planTrip()), DEBOUNCE_PLAN_TIME_MS)There was a problem hiding this comment.
the debounce time is a variable that comes from the store, so it could change. I updated my code so that it'll update the debounce function if that happens.
Codecov Report
@@ Coverage Diff @@
## dev #24 +/- ##
========================================
+ Coverage 7.43% 9.79% +2.35%
========================================
Files 36 36
Lines 619 623 +4
========================================
+ Hits 46 61 +15
+ Misses 573 562 -11
Continue to review full report at Codecov.
|
landonreed
left a comment
landonreed
left a comment
There was a problem hiding this comment.
Maybe it's just me, but I think the debounce logic is a little hard to follow. I'd like to see some comments in actions/form.js that describe what's going on a little more clearly.
|
I added some comments for you, @landonreed. lmk if it makes sense. |
