FREAC / LABINS

A web mapping application created for the Florida Department of Environmental Protection to provide survey data for the state of Florida.
https://labins.org/map
0 stars 0 forks source link

#131 this will correct the dropdown resetting #147

Closed clintonlunn closed 4 years ago

clintonlunn commented 4 years ago

Fixing issue #131

previously, county, city, quad dropdowns would not reset when selecting in the Township Range Section dropdowns. now, whenever Township, Range, Section dropdowns are selelected, all of the County, City, Quad dropdowns are reset.

In addition, another bug was fixed in this where the bufferlayer wasn't being cleared correctly when choosing county, city, or quad after choosing a township, range, section combination

clintonlunn commented 4 years ago

Thanks I’ll modify this code tonight

clintonlunn commented 4 years ago

Thanks I’ll modify this code tonight

clintonlunn commented 4 years ago

I did this and tested a little bit.

One thing I found was that the section dropdown should always be reset when township dropdown is changed and i found 2 ways of fixing it.

the 2 most recent commits:

  1. removes the .trs class from the section dropdown, this causes section dropdown even gets reset when the township or range dropdowns are selected (Range dropdown already reset the section dropdown because it had to rebuild the dropdown elements every time range dropdown val was changed
    • <select id="selectSection" class="form-control"></select>
  2. whenever range gets rebuilt, section also gets rebuilt. Basically whenever the township changes, range and section will be cleared.
    • for (j = sectionSelect.options.length - 1; j >= 0; j--) { sectionSelect.remove(j); }

I think approach 2 keeps the css classes more accurate since now we will be able to have the section dropdown have a class of 'trs', so that's good.

PhilipGriffith commented 4 years ago

Good points: I forgot that the logic in the TRS dropdowns depend on one another. :/ Here's a case I've been thinking about tied to 2: I choose a township, then a range, but then realize I got the township incorrect, so I go back and change it. But that act clears my range selection, which was correct, and now I have to redo it. And every time I click, we're making calls to the service.

How to fix? We probably talked about this a long time ago and said "tough luck!", but maybe this is a good time to revisit it.

Maybe we should just hardcode the township, range and section variables? This would make resetting things much simpler. And if an invalid combination is chosen, simply do nothing.

clintonlunn commented 4 years ago

Yeah there is a query made to rebuild the dropdown after each township. I think we had originally thought about making it work in both directions (range dropdown selection will repopulate the township dropdown choices and vice versa). Maybe this would be a good time to implement that.

I think the logic centered around only letting a user make a correct choice. From what I can tell, not all range values are the same depending on which township value you select. So, we don't want to display a range value that the user cannot access.

PhilipGriffith commented 4 years ago

You're right: we never wanted to let the user choose an invalid township (T)/range (R) combination. I think the answer will be to refactor the logic in the T dropdown into its own function that generalizes over either dropdown. Pseudo-code may look like this:

  1. When a dropdown is selected, check to see if the other dropdown has a value.
  2. If it does, include that in the query when searching for unique values with which to populate the dropdown.
  3. Make the query
  4. Build the dropdown with the results.

I think this will involve sending in the id of the selected dropdown. Knowing that, you immediately know what the other dropdown id should be.

clintonlunn commented 4 years ago

I think I get what you're saying

in short, you want the user to be able to input either value in first instead of the current workflow of: township --> range --> section

clintonlunn commented 4 years ago

thinking out loud here but in response to your last post:

What if you end up making a township/range dropdown that isn't valid? that would cause no section dropdown to get built.

example:

  1. I populate both township and range values on page load
  2. i choose township, which populates range
  3. then i change range with township already populated
  4. function doesn't know if query will return results, but since the township dropdown is populated, it runs the query, returns no results
  5. section dropdown isn't built thus, we've let the user choose a wrong value.

A possible solution to the above issue: whenever we see

  1. run a query to see if we return results (and only by count to speed it up),
  2. if we return results, a. we use that query to build the section dropdown. b. if we don't return results, we clear that dropdown and force the user to make a correct choice.

But, that would be we would pretty much need to run that query anytime that the user picks either township or range (after they've been initially populated with some search). I don't really see any performance problems with this but I also don't know if this is bad practice.

PhilipGriffith commented 4 years ago

I think we should just attempt to get data based on the information in the T and R (and optionally, the S) dropdowns: if nothing gets returned, nothing happens (or maybe a small information thing appears that says "TRS does not exist" or something to that effect).

This URL represents the kind of query you'd make if I initially chose "01E" from the R dropdown. It would build the T dropdown with 8 elements: https://maps.freac.fsu.edu/arcgis/rest/services/LABINS/LABINS_Data/MapServer/10/query?where=rng_ch+%3D+%2701%27+AND+rdir+%3D+%27E%27&text=&objectIds=&time=&geometry=&geometryType=esriGeometryEnvelope&inSR=&spatialRel=esriSpatialRelIntersects&relationParam=&outFields=twn_ch%2C+tdir&returnGeometry=false&returnTrueCurves=false&maxAllowableOffset=&geometryPrecision=&outSR=&returnIdsOnly=false&returnCountOnly=false&orderByFields=&groupByFieldsForStatistics=&outStatistics=&returnZ=false&returnM=false&gdbVersion=&returnDistinctValues=true&resultOffset=&resultRecordCount=&queryByDistance=&returnExtentsOnly=false&datumTransformation=&parameterValues=&rangeValues=&f=html

clintonlunn commented 4 years ago

This seemed simpler on the front end 🤦 . Sorry, still a work in progress..

PhilipGriffith commented 4 years ago

Don't overthink it. :)

clintonlunn commented 4 years ago

sucessfully building selects now. now just need to combine rangeselect and townshipselect events. sections are not hardcoded, but i don't think it will pose any problem

clintonlunn commented 4 years ago

Okay, I think we're looking pretty good on this. This is a net increase in lines of code, but for that we got better error handling and more flexibility on choosing the TRS combinations.

clintonlunn commented 4 years ago

I think I've addressed most/all of your suggestions. Thanks for giving me so much scrutiny, I need to change my way of thinking a bit. Please continue doing this (if your workday/time permits if course)!

Now, I think the next thing to do would be dynamic filling of the township/range dropdowns like you had alluded to previously.

PhilipGriffith commented 4 years ago

Thank you for contacting Florida State University. The following person(s) you are trying to contact directly or through a distribution list are no longer with the university. Philip Griffith (pdgriffith@fsu.edu) For further assistance, please contact the ITS Service Desk at 850-644-HELP (4357) or help.fsu.edu. This is an automated notification. Replies to this mailbox are not monitored.