[GitHub] zeppelin issue #1210: [ZEPPELIN-1220] Add geographical map as visualization ...

[corneadoug]

Github user corneadoug commented on the issue:

    https://github.com/apache/zeppelin/pull/1210
  
    @mwpenny 
    
    Alright, so after taking a look, here are my feedbacks:
    
    #### Overall PR
    * License of angular-esri-map will need to be added in [this 
file](https://github.com/apache/zeppelin/blob/master/zeppelin-distribution/src/bin_license/LICENSE)
 (That folder also contains single files if license needs to be copied)
    * There is 2 files to commit: `index.html` and `karma.conf.js` which have 
changes after we build.
    * I think it could be nice to have a page in the documentation, but let's 
make that in a different PR.
    
    #### The Map
    * The mouse scroll to zoom in the map should be deactivated, if you scroll 
down the notebook you might zoom out and then the map vanishes.
    * Could we have the map unzoomed to show the world when loaded (Its fine to 
keep it centered on Canada ;) )
    * The Map height do not scale when we make the paragraph bigger
    * The Map is going out of the paragraph instead of being contained or 
fitting the size:
    <img width="719" alt="screen shot 2016-08-02 at 5 03 20 pm" 
src="https://cloud.githubusercontent.com/assets/710411/17321366/31e0d1a8-58d3-11e6-8a17-e383f8912b3b.png";>
    * When I click on a Pin it moves me somewhere else
    
![mappin](https://cloud.githubusercontent.com/assets/710411/17321637/90e4d054-58d4-11e6-95ed-f8d2e63f78c7.gif)
    
    
    #### Options & Pivot
    * Let's remove the `height:36px` style for the pivot area, I know it was 
copy pasted from other place in the code, but its better to have all of them 
with the same height.
    * The BaseMap option isn't removed when we switch graphs, seems to be a 
mistake 
[here](https://github.com/mwpenny/zeppelin-esri/blob/esri/zeppelin-web/src/app/notebook/paragraph/paragraph-graphOptions.html#L31)
 with ng-if inside a class
    
    
    I will spend more time on checking the code, and would leave inline 
comments if there is


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---