google-code-export / sandy-disaster-recovery

Automatically exported from code.google.com/p/sandy-disaster-recovery
2 stars 2 forks source link

/map?id=$id&z=15 #71

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Create the ability to link directly to a work order.  The work order should be 
centered, and the Infobox should appear.  Default zoom should be 15.

After editing a work order, redirect to this URL.  When entering a work order, 
you should return to the blank form as it currently works.

Original issue reported on code.google.com by v...@aarontitus.net on 9 Nov 2012 at 9:38

GoogleCodeExporter commented 9 years ago
Attached is a Patch for this issue.

this patch allows to set request parameters in the /map path to directly zoom 
to a work order given an "id" request parameter. 

The infobox also shows up.

As per the description, the default zoom is 15 but can be changed via the "z" 
request parameter.

After editing a work order, the user is also redirected to the work order.

Original comment by vaf...@gmail.com on 27 Dec 2012 at 11:05

Attachments:

GoogleCodeExporter commented 9 years ago
Overall, this is good. A few comments:

In main.html, it would probably be better to just call ZoomSite at the end of 
initialize(). This means you need it in two places (both the demo and regular 
main), but keeps the main.html simpler.

In the ZoomSite function, you send an AJAX request for the new Site, which 
duplicates this logic. You don't update the siteMap, which means that searching 
for this site could yield incorrect results. It would be better to fix the 
loadSites function to handle this correctly. I would create the following 
wrappers:

1 - loadSites(url) - Takes a URL for the sites it should load (either the 
county version or the version for one or more IDs)
2 - loadSitesForCounty(county, status) - creates the correct URL, then calls 
loadSites(url)
3 - loadSitesById(ids) - creates the correct URL, then calls loadSites(url)

Then you won't have duplicated logic.

I'm sorry for the hassle! The code is complicated enough, and hopefully 
simplifying the loadSites functionality like this will make it easier to 
maintain.

I've added you as a contributor. Once you've made the changes I've mentioned 
above (unless you have a really good reason not to) go ahead and submit this 
change.

Original comment by rostovp...@gmail.com on 27 Dec 2012 at 11:32

GoogleCodeExporter commented 9 years ago
I'll study the code some more. The patch was a quick hack, i did it without 
understanding both the maps api and the current JS. 

Feel free ignore the patch or modify it, i'll keep an eye out on the svn 
changes.

Original comment by vaf...@gmail.com on 28 Dec 2012 at 12:10

GoogleCodeExporter commented 9 years ago
Hi rostovpack,

I'm having problem implementing your suggestion.

I have moved the code for ZoomSite at the end of initialize().

sandy.main.initialize = function(siteId, zoomLevel) {
  ...
  ...
  ...
  for (var i = 0; i < counties.length; ++i) {
    loadSitesForCounty(counties[i], "open");
  }
  goog.dom.getElement("open").checked = true;
  goog.dom.getElement("open").onclick = function() {
    goog.dom.getElement("open").onclick = null;
    for (var i = 0; i < counties.length; ++i) {
      loadSitesForCounty(counties[i], "closed");
    }
  }

  if (siteId) {
    zoomLevel = zoomLevel || 15;
    for(var siteName in siteMap) {
        if (siteId == siteMap[siteName]["id"])
        //.. do stuff
        // bug here... siteMap is still empty because loadSitesForCounty(counties[i], "open"); isnt done populating siteMap

    }
  }

Any suggestion how to fix this?

Original comment by vaf...@gmail.com on 28 Dec 2012 at 5:34

GoogleCodeExporter commented 9 years ago
Maybe allow a callback to be passed to the LoadSiteById() function, which will 
get called with the list of sites once they are all loaded. Then call 
LoadSiteById(siteId, some_callback) at the end of initialize(). some_callback 
will handle all of the zoom functionality.

Original comment by rostovp...@gmail.com on 28 Dec 2012 at 4:49

GoogleCodeExporter commented 9 years ago
Attached is patch that implements the callback in LoadSiteById.

Please review. Thanks.

Original comment by vaf...@gmail.com on 28 Dec 2012 at 7:03

Attachments:

GoogleCodeExporter commented 9 years ago
applied patch to implement 'id' and 'z' parameter

Original comment by vaf...@gmail.com on 1 Jan 2013 at 7:58