-
Notifications
You must be signed in to change notification settings - Fork 59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
make sure only the most recent request data is used #622
Conversation
src/Field/WfsSearch/WfsSearch.jsx
Outdated
this.setState({ | ||
fetching: true, | ||
latestRequestTime: fetchTime.getTime() | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure, if it may be relevant here, but updating the state might be asynchronous so the check in onFetchSuccess()
might fail in succession. It might be worthwhile to check to call the fetch in the callback from setState()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, of course. Fixed this and the failing test in 0ff62fc.
Very nice. I think it would additionally make sense to debounce the sending of the request while typing with the suggested smallish timeout. 100 ms probably? Only do thois if it can be done without hassle. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. Please merge.
I added a configurable timeout in 29d6b60, with a default of 300ms (that felt right to me, might be different for others). |
* Update dependencies * Cleanup import list * Readd commented line * Update src/GeometryUtil/GeometryUtil.js Co-authored-by: Marc Jansen <[email protected]> Co-authored-by: Marc Jansen <[email protected]>
BUGFIX
Description:
This fixes issue #621 , making sure only the most recent search request data is used. Thanks @KaiVolland !
@terrestris/devs Please review.