Skip to content
This repository has been archived by the owner. It is now read-only.

Include places in filter auto-suggest #45

Merged
merged 14 commits into from Sep 13, 2017

Conversation

Projects
None yet
2 participants
@c-w
Copy link
Member

commented Sep 13, 2017

Searching for locations

Switching search dataset

Searching for topics

Close-up of styling

@c-w c-w force-pushed the places-autosuggest branch from 8480f51 to 9e6d342 Sep 13, 2017

@@ -67,7 +67,6 @@ export const SERVICES = {

getHeatmapTiles(fromDate, toDate, zoomLevel, maintopic, tileid, periodType,
dataSource, externalsourceid, conjunctivetopics, callback) {
console.log(`processing tile request [${maintopic}, ${fromDate}, ${toDate}, ${tileid}}]`)

This comment has been minimized.

Copy link
@erikschlegel

erikschlegel Sep 13, 2017

Collaborator

can we leave this in for the moment as I depend on this for monitoring / debugging. this will be removed before we go live.

This comment has been minimized.

Copy link
@c-w

c-w Sep 13, 2017

Author Member

This creates a lot of noise and makes other debugging harder... but sure. 84969b0

suggestions={suggestions}
inputProps={inputProps}
focusInputOnSuggestionClick={true}
onSuggestionSelected={this.onSuggestionSelected}

This comment has been minimized.

Copy link
@erikschlegel

erikschlegel Sep 13, 2017

Collaborator

Agree that this should be there but you forgot to bind onSuggestionSelected to the component in the constructor.

This comment has been minimized.

Copy link
@c-w

c-w Sep 13, 2017

Author Member

This is not necessary given that the function now is an arrow function. You can learn more about how this works in this article (see "Approach 5: Arrow Function in Class Property").

It's much neater to use the arrow-function approach over the explicit bind approach (no duplicate code and potential to forget to bind the function) and it doesn't suffer from the performance overhead of re-declaring arrow function in the render method (this will cause a re-render of the component on every cycle since React has no way of knowing that the arrow function declared in render always is the same).

@@ -212,7 +215,8 @@ export default class SentimentTreeview extends React.Component {
</Subheader>
<div style={styles.searchBox}>
{ <TypeaheadSearch
dashboardRefreshFunc={(maintopic, conjunctivetopics)=>this.handleDataFetch(maintopic, conjunctivetopics)}
dashboardRefreshFunc={this.handleDataFetch}

This comment has been minimized.

Copy link
@erikschlegel

erikschlegel Sep 13, 2017

Collaborator

bind handleDataFetch

This comment has been minimized.

Copy link
@c-w

c-w Sep 13, 2017

Author Member

This is not necessary given that the function now is an arrow function. You can learn more about how this works in this article (see "Approach 5: Arrow Function in Class Property").

It's much neater to use the arrow-function approach over the explicit bind approach (no duplicate code and potential to forget to bind the function) and it doesn't suffer from the performance overhead of re-declaring arrow function in the render method (this will cause a re-render of the component on every cycle since React has no way of knowing that the arrow function declared in render always is the same).

'Term': 'fa fa-tag fa-2x',
};

function fetchLocationsFromFeatureService(bbox, matchName, callback) {

This comment has been minimized.

Copy link
@erikschlegel

erikschlegel Sep 13, 2017

Collaborator

can you move this function under the services folder

This comment has been minimized.

Copy link
@c-w

c-w Sep 13, 2017

Author Member

This is a fairly specific function so there won't be much potential for re-using it, but sure. 9fece04

@c-w

This comment has been minimized.

Copy link
Member Author

commented Sep 13, 2017

@erikschlegel NB: the arrow functions for binding work even on very old npm :)

image

@c-w c-w merged commit 75e14a4 into master Sep 13, 2017

@c-w c-w deleted the places-autosuggest branch Sep 13, 2017

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.