[Enhancement] Geocoder interaction improvements #1214
Conversation
heshan0131
commented
Aug 3, 2020
•
heshan0131
commented
- Refactor Geocoder to directly use mapbox client, and customize input dropdown components
- Simplify geocoder component style
- Move geocoder to top right
- Add flyto animation on result
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
|
@b2kdaman FYI |
heshan0131
changed the title
|
@heshan0131 gotta update tests |
| {!props.isExport && props.currentSample ? <SampleMapPanel {...props} /> : null} | ||
| <MapControl {...props} /> | ||
| <MapControl {...props} top={0} /> |
There was a problem hiding this comment.
One of them seems unnecessary
| margin-top: ${props => (props.placement === 'bottom' ? '4px' : 'auto')}; | ||
| margin-bottom: ${props => (props.placement === 'top' ? '4px' : 'auto')}; | ||
| margin-top: ${props => | ||
| props.placement === 'bottom' ? `${props.theme.dropdownWapperMargin}px` : 'auto'}; |
There was a problem hiding this comment.
${props.theme.dropdownWapperMargin}px
'px' should be part of dropdownWapperMargin
There was a problem hiding this comment.
actually, most of the sizes in kepler theme don't have a specified unit
src/components/geocoder-panel.js
Outdated
| import Geocoder from './geocoder/geocoder'; | ||
| import {GEOCODER_DATASET_NAME, GEOCODER_LAYER_ID} from 'constants/default-settings'; | ||
|
|
||
| const GEO_OFFSET = 0.05; |
There was a problem hiding this comment.
Move to geocoder/constants ?
| outline: none; | ||
| } | ||
| } | ||
| top: ${props => props.theme.geocoderTop}px; |
There was a problem hiding this comment.
Same with 'px'
Can you imagine if you will have to move from px to em someday?
There was a problem hiding this comment.
it's easy to do math without px, e.g. XXmargin = props.theme.geocoderTop + props.theme.sidepanelPadding
| }); | ||
| this.removeGeocoderDataset(); | ||
| GeocoderPanel.defaultProps = { | ||
| transitionDuration: 3000 |
There was a problem hiding this comment.
Probably should be calculated in future update. For example if I'm zoomed to Texas and first I look for Dallas an then Austin it would be annoying to wait 3 seconds for such transition.
| text, | ||
| bbox | ||
| } = geoItem; | ||
| this.removeGeocoderDataset(); |
There was a problem hiding this comment.
I thought we were trying to get away from it and I spent my time working on the solution to avoid creating new datasets/layers outside the mapbox. But I guess that's fine.
There was a problem hiding this comment.
this PR only applies cosmetic changes, e.g. pass actions instead of dispatch. your work on changing the rendering methods still applies
src/components/geocoder-panel.js
Outdated
| keepExistingConfig: true | ||
| }, | ||
| config: { | ||
| KeplerGlSchema.parseSavedConfig({ |
There was a problem hiding this comment.
can this be stored as a constant outside the class?
Signed-off-by: Shan He <heshan0131@gmail.com>
Signed-off-by: Shan He <heshan0131@gmail.com>
